[PATCH v2] apps: cam: Skip devices without connectors

[Milan Zamazal]

Hi Mattijs,

thank you for review.

Mattijs Korpershoek <mkorpershoek at kernel.org writes:

> Hi Milan,
>
> Thank you for the patch.
>
> On jeu., mai 15, 2025 at 18:25, Milan Zamazal <mzamazal at redhat.com> wrote:
>
>> Device::openCard() in the cam DRM helpers looks for a /dev/dri/card*
>> device that can be opened and that doesn't fail when asked about
>> DRM_CAP_DUMB_BUFFER capability (regardless whether the capability is
>> supported by the device).
>>
>> There can be matching devices that are not display devices.  This can
>> lead to selection of such a device and inability to use KMS output with
>> `cam' application.  The ultimate goal is to display something on the
>> device and later the KMS sink will fail if there is no connector
>> attached to the device (although it can actually fail earlier, when
>> trying to set DRM_CLIENT_CAP_ATOMIC capability if this is not
>> supported).  Let's avoid selecting devices without connectors.
>>
>> A question is whether the added check makes the check for
>> DRM_CAP_DUMB_BUFFER API redundant or not.
>>
>> Changes in v2:
>> - Rebased on current master.
>>
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>  src/apps/cam/drm.cpp | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/src/apps/cam/drm.cpp b/src/apps/cam/drm.cpp
>> index 47bbb6b0..e19e848c 100644
>> --- a/src/apps/cam/drm.cpp
>> +++ b/src/apps/cam/drm.cpp
>> @@ -479,6 +479,18 @@ int Device::openCard()
>>  			continue;
>>  		}
>>  
>> +		/* Skip devices without connectors. */
>
> This makes me think of what drm_hwcomposer does [1] with
> the IsKMSDev() check.
>
> The commit [2] introducing IsKMSDev() also gives some
> additional justification which might be useful to include (gpu devices
> being a different /dev/dri card).

I see, a good example, thank you.

>> +		std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
>> +			drmModeGetResources(fd_),
>> +			&drmModeFreeResources
>> +		};
>> +		if (!resources || resources->count_connectors <= 0) {
>
> Should we also check for count_crtcs and count_encoders ?

Based on the commit you mentioned, probably yes and it (still) works for
me.  Unless there are objections, I'll update the patch accordingly.

>> +			resources.reset();
>> +			drmClose(fd_);
>> +			fd_ = -1;
>> +			continue;
>> +		}
>> +
>
> Regards,
> Mattijs
>
> [1] https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/blob/main/drm/DrmDevice.cpp?ref_type=heads#L234
> [2] https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/commit/ec75ccd0735213423d6cf89409f8a3bfdaeddcee
>
>>  		found = true;
>>  		break;
>>  	}
>> -- 
>> 2.49.0