[PATCH v3] apps: cam: Skip non-display GPUs

[Milan Zamazal]

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> Quoting Milan Zamazal (2025-05-29 09:23:26)
>> 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, CRTCs or encoders.
>> The added check makes the original check for DRM_CAP_DUMB_BUFFER API
>> most likely unnecessary, let's remove it.
>> 
>> Changes in v3:
>> - Added checks for the presence of CRTCs and encoders.
>> - Removed the check for DRM_CAP_DUMB_BUFFER.
>> 
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>  src/apps/cam/drm.cpp | 22 ++++++++++------------
>>  1 file changed, 10 insertions(+), 12 deletions(-)
>> 
>> diff --git a/src/apps/cam/drm.cpp b/src/apps/cam/drm.cpp
>> index 47bbb6b05..c40c31410 100644
>> --- a/src/apps/cam/drm.cpp
>> +++ b/src/apps/cam/drm.cpp
>> @@ -450,8 +450,6 @@ int Device::openCard()
>>         }
>>  
>>         for (struct dirent *res; (res = readdir(folder));) {
>> -               uint64_t cap;
>> -
>>                 if (strncmp(res->d_name, "card", 4))
>>                         continue;
>>  
>> @@ -464,16 +462,16 @@ int Device::openCard()
>>                         continue;
>>                 }
>>  
>> -               /*
>> -                * Skip devices that don't support the modeset API, to avoid
>> -                * selecting a DRM device corresponding to a GPU. There is no
>> -                * modeset capability, but the kernel returns an error for most
>> -                * caps if mode setting isn't support by the driver. The
>> -                * DRM_CAP_DUMB_BUFFER capability is one of those, other would
>> -                * do as well. The capability value itself isn't relevant.
>> -                */
>> -               ret = drmGetCap(fd_, DRM_CAP_DUMB_BUFFER, &cap);
>> -               if (ret < 0) {
>> +               /* Skip devices without connectors. */
>
> This all sounds reasonable to me, though this comment now skips devices
> without connectors/crts/encoders.
>
> Perhaps we should update it while applying?
>
> 		/* Skip non-display devices */

Ah, yes, please.

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>> +               std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
>> +                       drmModeGetResources(fd_),
>> +                       &drmModeFreeResources
>> +               };
>> +               if (!resources ||
>> +                   resources->count_connectors <= 0 ||
>> +                   resources->count_crtcs <= 0 ||
>> +                   resources->count_encoders <= 0) {
>> +                       resources.reset();
>>                         drmClose(fd_);
>>                         fd_ = -1;
>>                         continue;
>> -- 
>> 2.49.0
>>