[v2] apps: cam: Skip devices without connectors
diff mbox series

Message ID 20250515162536.131131-1-mzamazal@redhat.com
State Superseded
Headers show
Series
  • [v2] apps: cam: Skip devices without connectors
Related show

Commit Message

Milan Zamazal May 15, 2025, 4:25 p.m. UTC
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@redhat.com>
---
 src/apps/cam/drm.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Laurent Pinchart May 15, 2025, 10:27 p.m. UTC | #1
Hi Milan,

Thank you for the patch.

On Thu, May 15, 2025 at 06:25:36PM +0200, Milan Zamazal 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

Can you share an example of such a device ? Are we talking about a
device that sets DRIVER_MODESET but is not a display device ?

> 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@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. */
> +		std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
> +			drmModeGetResources(fd_),
> +			&drmModeFreeResources
> +		};
> +		if (!resources || resources->count_connectors <= 0) {
> +			resources.reset();
> +			drmClose(fd_);
> +			fd_ = -1;
> +			continue;
> +		}
> +
>  		found = true;
>  		break;
>  	}
Milan Zamazal May 16, 2025, 3:15 p.m. UTC | #2
Hi Laurent,

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Milan,
>
> Thank you for the patch.
>
> On Thu, May 15, 2025 at 06:25:36PM +0200, Milan Zamazal 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
>
> Can you share an example of such a device ? Are we talking about a
> device that sets DRIVER_MODESET but is not a display device ?

I guess so.  I have the following on TI AM69:

  # ls -l /dev/dri/
  total 0
  drwxr-xr-x 2 root root        100 Mar  7 01:00 by-path/
  crw-rw---- 1 root video  226,   0 Mar  7 01:00 card0
  crw-rw---- 1 root video  226,   1 Mar  7 01:00 card1
  crw-rw-rw- 1 root render 226, 128 Mar  7 01:00 renderD128

  # ls -l /sys/class/drm/
  total 0
  lrwxrwxrwx 1 root root    0 Mar  7 01:00 card0 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0'/
  lrwxrwxrwx 1 root root    0 Mar  7 01:00 card0-DP-1 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0/card0-DP-1'/
  lrwxrwxrwx 1 root root    0 Mar  7 01:00 card0-HDMI-A-1 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0/card0-HDMI-A-1'/
  lrwxrwxrwx 1 root root    0 May 16 14:31 card1 -> '../../devices/platform/bus@100000/4e20000000.gpu/drm/card1'/
  lrwxrwxrwx 1 root root    0 May 16 14:31 renderD128 -> '../../devices/platform/bus@100000/4e20000000.gpu/drm/renderD128'/
  -r--r--r-- 1 root root 4096 May 16 16:49 version

card1 causes the problem.

>> 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@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. */
>> +		std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
>> +			drmModeGetResources(fd_),
>> +			&drmModeFreeResources
>> +		};
>> +		if (!resources || resources->count_connectors <= 0) {
>> +			resources.reset();
>> +			drmClose(fd_);
>> +			fd_ = -1;
>> +			continue;
>> +		}
>> +
>>  		found = true;
>>  		break;
>>  	}
Hi Milan,

Thank you for the patch.

On jeu., mai 15, 2025 at 18:25, Milan Zamazal <mzamazal@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@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).

> +		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 ?

> +			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
Mattijs Korpershoek May 28, 2025, 1:25 p.m. UTC | #4
On mer., mai 28, 2025 at 13:30, Mattijs@ideasonboard.com,	"Korpershoek <mkorpershoek"@kernel.org wrote:

I don't know what went wrong, but I think I forgot a trailing '>' in
the From: section of the email.

Sorry about that.

> Hi Milan,
>
> Thank you for the patch.
>
> On jeu., mai 15, 2025 at 18:25, Milan Zamazal <mzamazal@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@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).
>
>> +		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 ?
>
>> +			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
Milan Zamazal May 28, 2025, 2:27 p.m. UTC | #5
Hi Mattijs,

thank you for review.

Mattijs Korpershoek <mkorpershoek@kernel.org writes:

> Hi Milan,
>
> Thank you for the patch.
>
> On jeu., mai 15, 2025 at 18:25, Milan Zamazal <mzamazal@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@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
Laurent Pinchart May 28, 2025, 2:37 p.m. UTC | #6
On Fri, May 16, 2025 at 05:15:48PM +0200, Milan Zamazal wrote:
> Laurent Pinchart writes:
> > On Thu, May 15, 2025 at 06:25:36PM +0200, Milan Zamazal 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
> >
> > Can you share an example of such a device ? Are we talking about a
> > device that sets DRIVER_MODESET but is not a display device ?
> 
> I guess so.  I have the following on TI AM69:
> 
>   # ls -l /dev/dri/
>   total 0
>   drwxr-xr-x 2 root root        100 Mar  7 01:00 by-path/
>   crw-rw---- 1 root video  226,   0 Mar  7 01:00 card0
>   crw-rw---- 1 root video  226,   1 Mar  7 01:00 card1
>   crw-rw-rw- 1 root render 226, 128 Mar  7 01:00 renderD128
> 
>   # ls -l /sys/class/drm/
>   total 0
>   lrwxrwxrwx 1 root root    0 Mar  7 01:00 card0 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0'/
>   lrwxrwxrwx 1 root root    0 Mar  7 01:00 card0-DP-1 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0/card0-DP-1'/
>   lrwxrwxrwx 1 root root    0 Mar  7 01:00 card0-HDMI-A-1 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0/card0-HDMI-A-1'/
>   lrwxrwxrwx 1 root root    0 May 16 14:31 card1 -> '../../devices/platform/bus@100000/4e20000000.gpu/drm/card1'/
>   lrwxrwxrwx 1 root root    0 May 16 14:31 renderD128 -> '../../devices/platform/bus@100000/4e20000000.gpu/drm/renderD128'/
>   -r--r--r-- 1 root root 4096 May 16 16:49 version
> 
> card1 causes the problem.

Right. The usual story of out-of-tree drivers being full of bugs. One
day I'll tackle the fundamental unfairness of upstream developers having
to suffer from bugs of non-upstream code. After tackling the other
unfairness that we software developers always have to suffer from
hardware bugs, and never the other way around.

Skipping devices without connectors seems to be the best workaround.
Should we drop the DRM_CAP_DUMB_BUFFER capability query too ?

> >> 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@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. */
> >> +		std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
> >> +			drmModeGetResources(fd_),
> >> +			&drmModeFreeResources
> >> +		};
> >> +		if (!resources || resources->count_connectors <= 0) {
> >> +			resources.reset();
> >> +			drmClose(fd_);
> >> +			fd_ = -1;
> >> +			continue;
> >> +		}
> >> +
> >>  		found = true;
> >>  		break;
> >>  	}
Laurent Pinchart May 28, 2025, 2:41 p.m. UTC | #7
On Wed, May 28, 2025 at 04:27:36PM +0200, Milan Zamazal wrote:
> Mattijs Korpershoek writes:
> > On jeu., mai 15, 2025 at 18:25, Milan Zamazal 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@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.

Sounds good to me.

It would be nice to avoid querying resources twice, but it's not a very
big deal.

> >> +			resources.reset();
> >> +			drmClose(fd_);
> >> +			fd_ = -1;
> >> +			continue;
> >> +		}
> >> +
> >
> > [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;
> >>  	}
Milan Zamazal May 28, 2025, 5:41 p.m. UTC | #8
Hi Laurent,

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> On Fri, May 16, 2025 at 05:15:48PM +0200, Milan Zamazal wrote:
>> Laurent Pinchart writes:
>> > On Thu, May 15, 2025 at 06:25:36PM +0200, Milan Zamazal 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
>> >
>> > Can you share an example of such a device ? Are we talking about a
>> > device that sets DRIVER_MODESET but is not a display device ?
>> 
>> I guess so.  I have the following on TI AM69:
>> 
>>   # ls -l /dev/dri/
>>   total 0
>>   drwxr-xr-x 2 root root        100 Mar  7 01:00 by-path/
>>   crw-rw---- 1 root video  226,   0 Mar  7 01:00 card0
>>   crw-rw---- 1 root video  226,   1 Mar  7 01:00 card1
>>   crw-rw-rw- 1 root render 226, 128 Mar  7 01:00 renderD128
>> 
>>   # ls -l /sys/class/drm/
>>   total 0
>>   lrwxrwxrwx 1 root root 0 Mar 7 01:00 card0 -> '../../devices/platform/bus@100000/4a00000.dss/drm/card0'/
>>   lrwxrwxrwx 1 root root 0 Mar 7 01:00 card0-DP-1 ->
>> '../../devices/platform/bus@100000/4a00000.dss/drm/card0/card0-DP-1'/
>>   lrwxrwxrwx 1 root root 0 Mar 7 01:00 card0-HDMI-A-1 ->
>> '../../devices/platform/bus@100000/4a00000.dss/drm/card0/card0-HDMI-A-1'/
>>   lrwxrwxrwx 1 root root 0 May 16 14:31 card1 -> '../../devices/platform/bus@100000/4e20000000.gpu/drm/card1'/
>>   lrwxrwxrwx 1 root root 0 May 16 14:31 renderD128 ->
>> '../../devices/platform/bus@100000/4e20000000.gpu/drm/renderD128'/
>>   -r--r--r-- 1 root root 4096 May 16 16:49 version
>> 
>> card1 causes the problem.
>
> Right. The usual story of out-of-tree drivers being full of bugs. One
> day I'll tackle the fundamental unfairness of upstream developers having
> to suffer from bugs of non-upstream code. After tackling the other
> unfairness that we software developers always have to suffer from
> hardware bugs, and never the other way around.

There are reasons why I avoid touching hardware and proprietary software
to the extent possible.

> Skipping devices without connectors seems to be the best workaround.
> Should we drop the DRM_CAP_DUMB_BUFFER capability query too ?

The check looks redundant to me now.  I'm not an expert but let's try to
drop it.

>> >> 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@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. */
>> >> +		std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
>> >> +			drmModeGetResources(fd_),
>> >> +			&drmModeFreeResources
>> >> +		};
>> >> +		if (!resources || resources->count_connectors <= 0) {
>> >> +			resources.reset();
>> >> +			drmClose(fd_);
>> >> +			fd_ = -1;
>> >> +			continue;
>> >> +		}
>> >> +
>> >>  		found = true;
>> >>  		break;
>> >>  	}

Patch
diff mbox series

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. */
+		std::unique_ptr<drmModeRes, decltype(&drmModeFreeResources)> resources{
+			drmModeGetResources(fd_),
+			&drmModeFreeResources
+		};
+		if (!resources || resources->count_connectors <= 0) {
+			resources.reset();
+			drmClose(fd_);
+			fd_ = -1;
+			continue;
+		}
+
 		found = true;
 		break;
 	}