[PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[José Roberto de Souza]

All DRM_CLIENT capabilities are tied to KMS support, so returning
-ENOTSUPP when KMS is not supported.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..842423fe9762 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -306,6 +306,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 {
 	struct drm_set_client_cap *req = data;
 
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -ENOTSUPP;
+
 	switch (req->capability) {
 	case DRM_CLIENT_CAP_STEREO_3D:
 		if (req->value > 1)
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[Patchwork]

== Series Details ==

Series: drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS
URL   : https://patchwork.freedesktop.org/series/49675/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4820 -> Patchwork_10181 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49675/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10181 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-ilk-650:         DMESG-WARN (fdo#106387) -> PASS

    igt@pm_rpm@module-reload:
      {fi-cnl-u}:         FAIL -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (49 -> 45) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4820 -> Patchwork_10181

  CI_DRM_4820: a804643cba4646e18e0e31c9363fdd3c92dca3c0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10181: c32bb0a966f6d838450db44d9c5c0dad28373fb3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c32bb0a966f6 drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10181/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[Chris Wilson]

Quoting José Roberto de Souza (2018-09-13 23:13:41)
> All DRM_CLIENT capabilities are tied to KMS support, so returning
> -ENOTSUPP when KMS is not supported.

The posix errno is ENOTSUP (ENOTSUPP is internal). Now since we have no
ENOTSUP in the uapi, I've switched to using EOPNOTSUP as that is
documented to have the same value as ENOTSUP under Linux. (At least
until somebody decided to make ENOTSUP unique.)

> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 6b4a633b4240..842423fe9762 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -306,6 +306,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>  {
>         struct drm_set_client_cap *req = data;
>  
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -ENOTSUPP;

The wider question though is client cap restricted to modesetting
capabilities? Or should each case include a check like
DRM_CLIENT_CAP_ATOMIC.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[Souza, Jose]

On Fri, 2018-09-14 at 09:15 +0100, Chris Wilson wrote:
> Quoting José Roberto de Souza (2018-09-13 23:13:41)
> > All DRM_CLIENT capabilities are tied to KMS support, so returning
> > -ENOTSUPP when KMS is not supported.
> 
> The posix errno is ENOTSUP (ENOTSUPP is internal). Now since we have
> no
> ENOTSUP in the uapi, I've switched to using EOPNOTSUP as that is
> documented to have the same value as ENOTSUP under Linux. (At least
> until somebody decided to make ENOTSUP unique.)

Oh thanks, I have copied it from drm_getcap() and did not notice.

> 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_ioctl.c
> > b/drivers/gpu/drm/drm_ioctl.c
> > index 6b4a633b4240..842423fe9762 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -306,6 +306,9 @@ drm_setclientcap(struct drm_device *dev, void
> > *data, struct drm_file *file_priv)
> >  {
> >         struct drm_set_client_cap *req = data;
> >  
> > +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > +               return -ENOTSUPP;
> 
> The wider question though is client cap restricted to modesetting
> capabilities? Or should each case include a check like
> DRM_CLIENT_CAP_ATOMIC.

Well all of those:

DRM_CLIENT_CAP_STEREO_3D
DRM_CLIENT_CAP_UNIVERSAL_PLANES
DRM_CLIENT_CAP_ATOMIC
DRM_CLIENT_CAP_ASPECT_RATIO
DRM_CLIENT_CAP_WRITEBACK_CONNECTORS

are just usefull with KMS.


> -Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[Chris Wilson]

Quoting Souza, Jose (2018-09-14 17:30:59)
> On Fri, 2018-09-14 at 09:15 +0100, Chris Wilson wrote:
> > Quoting José Roberto de Souza (2018-09-13 23:13:41)
> > > @@ -306,6 +306,9 @@ drm_setclientcap(struct drm_device *dev, void
> > > *data, struct drm_file *file_priv)
> > >  {
> > >         struct drm_set_client_cap *req = data;
> > >  
> > > +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > > +               return -ENOTSUPP;
> > 
> > The wider question though is client cap restricted to modesetting
> > capabilities? Or should each case include a check like
> > DRM_CLIENT_CAP_ATOMIC.
> 
> Well all of those:
> 
> DRM_CLIENT_CAP_STEREO_3D
> DRM_CLIENT_CAP_UNIVERSAL_PLANES
> DRM_CLIENT_CAP_ATOMIC
> DRM_CLIENT_CAP_ASPECT_RATIO
> DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
> 
> are just usefull with KMS.

It more about the semantics. If it's the first guard in the function, it
gives the impression that the setclientcap ioctl is KMS only. If they
are repeated for each case, then it's clear that the ioctl is more
general and it just those caps that are KMS only.

Imo, the drm_client is wider than the kms interface, but I may be wrong.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[Daniel Vetter]

On Fri, Sep 14, 2018 at 7:04 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Souza, Jose (2018-09-14 17:30:59)
>> On Fri, 2018-09-14 at 09:15 +0100, Chris Wilson wrote:
>> > Quoting José Roberto de Souza (2018-09-13 23:13:41)
>> > > @@ -306,6 +306,9 @@ drm_setclientcap(struct drm_device *dev, void
>> > > *data, struct drm_file *file_priv)
>> > >  {
>> > >         struct drm_set_client_cap *req = data;
>> > >
>> > > +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
>> > > +               return -ENOTSUPP;
>> >
>> > The wider question though is client cap restricted to modesetting
>> > capabilities? Or should each case include a check like
>> > DRM_CLIENT_CAP_ATOMIC.
>>
>> Well all of those:
>>
>> DRM_CLIENT_CAP_STEREO_3D
>> DRM_CLIENT_CAP_UNIVERSAL_PLANES
>> DRM_CLIENT_CAP_ATOMIC
>> DRM_CLIENT_CAP_ASPECT_RATIO
>> DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
>>
>> are just usefull with KMS.
>
> It more about the semantics. If it's the first guard in the function, it
> gives the impression that the setclientcap ioctl is KMS only. If they
> are repeated for each case, then it's clear that the ioctl is more
> general and it just those caps that are KMS only.
>
> Imo, the drm_client is wider than the kms interface, but I may be wrong.

In getcap we have 2 blocks, with DRIVER_MODESET check in between. I
think a comment along the lines of


> -Chris
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Return -ENOTSUPP in drm_setclientcap() when driver do not support KMS

[Daniel Vetter]

On Fri, Sep 14, 2018 at 10:02 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Sep 14, 2018 at 7:04 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Quoting Souza, Jose (2018-09-14 17:30:59)
>>> On Fri, 2018-09-14 at 09:15 +0100, Chris Wilson wrote:
>>> > Quoting José Roberto de Souza (2018-09-13 23:13:41)
>>> > > @@ -306,6 +306,9 @@ drm_setclientcap(struct drm_device *dev, void
>>> > > *data, struct drm_file *file_priv)
>>> > >  {
>>> > >         struct drm_set_client_cap *req = data;
>>> > >
>>> > > +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
>>> > > +               return -ENOTSUPP;
>>> >
>>> > The wider question though is client cap restricted to modesetting
>>> > capabilities? Or should each case include a check like
>>> > DRM_CLIENT_CAP_ATOMIC.
>>>
>>> Well all of those:
>>>
>>> DRM_CLIENT_CAP_STEREO_3D
>>> DRM_CLIENT_CAP_UNIVERSAL_PLANES
>>> DRM_CLIENT_CAP_ATOMIC
>>> DRM_CLIENT_CAP_ASPECT_RATIO
>>> DRM_CLIENT_CAP_WRITEBACK_CONNECTORS
>>>
>>> are just usefull with KMS.
>>
>> It more about the semantics. If it's the first guard in the function, it
>> gives the impression that the setclientcap ioctl is KMS only. If they
>> are repeated for each case, then it's clear that the ioctl is more
>> general and it just those caps that are KMS only.
>>
>> Imo, the drm_client is wider than the kms interface, but I may be wrong.

Oops, slipped :-)

In getcap we have 2 blocks, with DRIVER_MODESET check in between. I
think a comment along the lines of

/* No render-only settable capabilities for now */

/* Below caps only work with KMS drivers */
if (!drm_core_check_feature(DRIVER_MODESET))
   return -ENOTSUPP;

I think with that it's clear that it's _not_ the top-level check, and
if someone needs to add a gem cap, they know where to put it. Not that
we needed that anytime in the past 10 years, but who knows.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx