[PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis

[PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We now have per-device driver_features, so let's use that
to disable atomic only for pre-nv50.

Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: nouveau@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 70dce544984e..670535a68d3b 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev)
 	nouveau_display(dev)->fini = nv04_display_fini;
 
 	/* Pre-nv50 doesn't support atomic, so don't expose the ioctls */
-	dev->driver->driver_features &= ~DRIVER_ATOMIC;
+	dev->driver_features &= ~DRIVER_ATOMIC;
 
 	nouveau_hw_save_vga_fonts(dev, 1);
 
-- 
2.16.4

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

[PATCH 2/2] drm/amdgpu: Use per-device driver_features to disable atomic

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Disable atomic on a per-device basis instead of for all devices.
Made possible by the new device.driver_features thing.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Michel Dänzer <michel@daenzer.net>
Suggested-by: Michel Dänzer <michel@daenzer.net>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6870909da926..8c1db96be070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -816,17 +816,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	/* warn the user if they mix atomic and non-atomic capable GPUs */
-	if ((kms_driver.driver_features & DRIVER_ATOMIC) && !supports_atomic)
-		DRM_ERROR("Mixing atomic and non-atomic capable GPUs!\n");
-	/* support atomic early so the atomic debugfs stuff gets created */
-	if (supports_atomic)
-		kms_driver.driver_features |= DRIVER_ATOMIC;
-
 	dev = drm_dev_alloc(&kms_driver, &pdev->dev);
 	if (IS_ERR(dev))
 		return PTR_ERR(dev);
 
+	if (!supports_atomic)
+		dev->driver_features &= ~DRIVER_ATOMIC;
+
 	ret = pci_enable_device(pdev);
 	if (ret)
 		goto err_free;
@@ -1078,7 +1074,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
 
 static struct drm_driver kms_driver = {
 	.driver_features =
-	    DRIVER_USE_AGP |
+	    DRIVER_USE_AGP | DRIVER_ATOMIC |
 	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
 	    DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
 	.load = amdgpu_driver_load_kms,
-- 
2.16.4

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

Re: [PATCH 2/2] drm/amdgpu: Use per-device driver_features to disable atomic

[Michel Dänzer]

On 2018-09-13 6:31 p.m., Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Disable atomic on a per-device basis instead of for all devices.
> Made possible by the new device.driver_features thing.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Michel Dänzer <michel@daenzer.net>
> Suggested-by: Michel Dänzer <michel@daenzer.net>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6870909da926..8c1db96be070 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -816,17 +816,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>  	if (ret)
>  		return ret;
>  
> -	/* warn the user if they mix atomic and non-atomic capable GPUs */
> -	if ((kms_driver.driver_features & DRIVER_ATOMIC) && !supports_atomic)
> -		DRM_ERROR("Mixing atomic and non-atomic capable GPUs!\n");
> -	/* support atomic early so the atomic debugfs stuff gets created */
> -	if (supports_atomic)
> -		kms_driver.driver_features |= DRIVER_ATOMIC;
> -
>  	dev = drm_dev_alloc(&kms_driver, &pdev->dev);
>  	if (IS_ERR(dev))
>  		return PTR_ERR(dev);
>  
> +	if (!supports_atomic)
> +		dev->driver_features &= ~DRIVER_ATOMIC;
> +
>  	ret = pci_enable_device(pdev);
>  	if (ret)
>  		goto err_free;
> @@ -1078,7 +1074,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
>  
>  static struct drm_driver kms_driver = {
>  	.driver_features =
> -	    DRIVER_USE_AGP |
> +	    DRIVER_USE_AGP | DRIVER_ATOMIC |
>  	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
>  	    DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
>  	.load = amdgpu_driver_load_kms,
> 

Thanks Ville for taking care of this!

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>

If Alex agrees, probably best to push this to drm-misc-next as well.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/amdgpu: Use per-device driver_features to disable atomic

[Alex Deucher]

On Thu, Sep 13, 2018 at 12:39 PM Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2018-09-13 6:31 p.m., Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Disable atomic on a per-device basis instead of for all devices.
> > Made possible by the new device.driver_features thing.
> >
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: Harry Wentland <harry.wentland@amd.com>
> > Cc: Michel Dänzer <michel@daenzer.net>
> > Suggested-by: Michel Dänzer <michel@daenzer.net>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 6870909da926..8c1db96be070 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -816,17 +816,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> >       if (ret)
> >               return ret;
> >
> > -     /* warn the user if they mix atomic and non-atomic capable GPUs */
> > -     if ((kms_driver.driver_features & DRIVER_ATOMIC) && !supports_atomic)
> > -             DRM_ERROR("Mixing atomic and non-atomic capable GPUs!\n");
> > -     /* support atomic early so the atomic debugfs stuff gets created */
> > -     if (supports_atomic)
> > -             kms_driver.driver_features |= DRIVER_ATOMIC;
> > -
> >       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> >       if (IS_ERR(dev))
> >               return PTR_ERR(dev);
> >
> > +     if (!supports_atomic)
> > +             dev->driver_features &= ~DRIVER_ATOMIC;
> > +
> >       ret = pci_enable_device(pdev);
> >       if (ret)
> >               goto err_free;
> > @@ -1078,7 +1074,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
> >
> >  static struct drm_driver kms_driver = {
> >       .driver_features =
> > -         DRIVER_USE_AGP |
> > +         DRIVER_USE_AGP | DRIVER_ATOMIC |
> >           DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
> >           DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
> >       .load = amdgpu_driver_load_kms,
> >
>
> Thanks Ville for taking care of this!
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>
> If Alex agrees, probably best to push this to drm-misc-next as well.
>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Please go ahead and merge through drm-misc.

Thanks!

Alex

>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/amdgpu: Use per-device driver_features to disable atomic

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 12:40:20PM -0400, Alex Deucher wrote:
> On Thu, Sep 13, 2018 at 12:39 PM Michel Dänzer <michel@daenzer.net> wrote:
> >
> > On 2018-09-13 6:31 p.m., Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Disable atomic on a per-device basis instead of for all devices.
> > > Made possible by the new device.driver_features thing.
> > >
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > > Cc: Harry Wentland <harry.wentland@amd.com>
> > > Cc: Michel Dänzer <michel@daenzer.net>
> > > Suggested-by: Michel Dänzer <michel@daenzer.net>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++--------
> > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > index 6870909da926..8c1db96be070 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > @@ -816,17 +816,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> > >       if (ret)
> > >               return ret;
> > >
> > > -     /* warn the user if they mix atomic and non-atomic capable GPUs */
> > > -     if ((kms_driver.driver_features & DRIVER_ATOMIC) && !supports_atomic)
> > > -             DRM_ERROR("Mixing atomic and non-atomic capable GPUs!\n");
> > > -     /* support atomic early so the atomic debugfs stuff gets created */
> > > -     if (supports_atomic)
> > > -             kms_driver.driver_features |= DRIVER_ATOMIC;
> > > -
> > >       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> > >       if (IS_ERR(dev))
> > >               return PTR_ERR(dev);
> > >
> > > +     if (!supports_atomic)
> > > +             dev->driver_features &= ~DRIVER_ATOMIC;
> > > +
> > >       ret = pci_enable_device(pdev);
> > >       if (ret)
> > >               goto err_free;
> > > @@ -1078,7 +1074,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
> > >
> > >  static struct drm_driver kms_driver = {
> > >       .driver_features =
> > > -         DRIVER_USE_AGP |
> > > +         DRIVER_USE_AGP | DRIVER_ATOMIC |
> > >           DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
> > >           DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
> > >       .load = amdgpu_driver_load_kms,
> > >
> >
> > Thanks Ville for taking care of this!
> >
> > Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
> >
> > If Alex agrees, probably best to push this to drm-misc-next as well.
> >
> 
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> 
> Please go ahead and merge through drm-misc.
> 
> Thanks!

You are welcome, and thanks for the r-bs. Pushed.

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

Re: [PATCH 2/2] drm/amdgpu: Use per-device driver_features to disable atomic

[Harry Wentland]

On 2018-09-13 12:31 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Disable atomic on a per-device basis instead of for all devices.
> Made possible by the new device.driver_features thing.
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Michel Dänzer <michel@daenzer.net>
> Suggested-by: Michel Dänzer <michel@daenzer.net>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6870909da926..8c1db96be070 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -816,17 +816,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>  	if (ret)
>  		return ret;
>  
> -	/* warn the user if they mix atomic and non-atomic capable GPUs */
> -	if ((kms_driver.driver_features & DRIVER_ATOMIC) && !supports_atomic)
> -		DRM_ERROR("Mixing atomic and non-atomic capable GPUs!\n");
> -	/* support atomic early so the atomic debugfs stuff gets created */
> -	if (supports_atomic)
> -		kms_driver.driver_features |= DRIVER_ATOMIC;
> -
>  	dev = drm_dev_alloc(&kms_driver, &pdev->dev);
>  	if (IS_ERR(dev))
>  		return PTR_ERR(dev);
>  
> +	if (!supports_atomic)
> +		dev->driver_features &= ~DRIVER_ATOMIC;
> +
>  	ret = pci_enable_device(pdev);
>  	if (ret)
>  		goto err_free;
> @@ -1078,7 +1074,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
>  
>  static struct drm_driver kms_driver = {
>  	.driver_features =
> -	    DRIVER_USE_AGP |
> +	    DRIVER_USE_AGP | DRIVER_ATOMIC |
>  	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
>  	    DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
>  	.load = amdgpu_driver_load_kms,
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/nouveau: Disable atomic support on a per-device basis

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/nouveau: Disable atomic support on a per-device basis
URL   : https://patchwork.freedesktop.org/series/49650/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4817 -> Patchwork_10172 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10172 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10172, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10172:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@mock_hugepages:
      fi-bwr-2160:        PASS -> DMESG-FAIL

    
    ==== Warnings ====

    igt@pm_rpm@module-reload:
      fi-hsw-4770r:       SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-kbl-soraka:      NOTRUN -> INCOMPLETE (fdo#107774, fdo#107556)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       NOTRUN -> INCOMPLETE (fdo#107718)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-WARN (fdo#102614)
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload-inject:
      fi-hsw-4770r:       DMESG-WARN -> PASS

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

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

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128, fdo#107139) -> PASS

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

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774


== Participating hosts (48 -> 44) ==

  Additional (1): fi-kbl-soraka 
  Missing    (5): fi-byt-j1900 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4817 -> Patchwork_10172

  CI_DRM_4817: 0725fa5c5756ff86de0f33f10b9d92ac452d5118 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10172: f57b35508efe08d2294d622ac5a93d2113c04a4e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f57b35508efe drm/amdgpu: Use per-device driver_features to disable atomic
c47608435df1 drm/nouveau: Disable atomic support on a per-device basis

== Logs ==

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

Re: [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis

[Lyude Paul]

Hm, one nitpick here. Since /sys/kernel/debug/dri/*/state creation depends on
the driver supporting atomic, maybe it would be good to make it so that we set
DRIVER_ATOMIC in the driver_stub structure, then disable it per-device depending
on the nouveau_atomic setting + hw support. That way we can always have the
state debugfs file while maintaining nouveau's current behavior with exposing
atomic ioctls. Assuming that wouldn't have any unintended side-effects of course

On Thu, 2018-09-13 at 19:31 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We now have per-device driver_features, so let's use that
> to disable atomic only for pre-nv50.
> 
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: nouveau@lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index 70dce544984e..670535a68d3b 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev)
>  	nouveau_display(dev)->fini = nv04_display_fini;
>  
>  	/* Pre-nv50 doesn't support atomic, so don't expose the ioctls */
> -	dev->driver->driver_features &= ~DRIVER_ATOMIC;
> +	dev->driver_features &= ~DRIVER_ATOMIC;
>  
>  	nouveau_hw_save_vga_fonts(dev, 1);
>  

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis

[Daniel Vetter]

On Thu, Sep 13, 2018 at 11:02 PM, Lyude Paul <lyude@redhat.com> wrote:
> Hm, one nitpick here. Since /sys/kernel/debug/dri/*/state creation depends on
> the driver supporting atomic, maybe it would be good to make it so that we set
> DRIVER_ATOMIC in the driver_stub structure, then disable it per-device depending
> on the nouveau_atomic setting + hw support. That way we can always have the
> state debugfs file while maintaining nouveau's current behavior with exposing
> atomic ioctls. Assuming that wouldn't have any unintended side-effects of course

dri/*/state only works with atomic drivers. There's no explicit state
with legacy drivers at all, it's all just implicit in hw and some
random driver structures.

We should make sure though that the debugfs stuff looks at
drm_drv_uses_atomic_modsetting(), and not DRIVER_ATOMIC. Former is
about the internals (i915 is internally atomic everywhere), latter
about the uapi (some old platforms aren't properly validated for full
atomic features, hence why it's disabled).
-Daniel

> On Thu, 2018-09-13 at 19:31 +0300, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> We now have per-device driver_features, so let's use that
>> to disable atomic only for pre-nv50.
>>
>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> Cc: Lyude Paul <lyude@redhat.com>
>> Cc: nouveau@lists.freedesktop.org
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
>> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
>> index 70dce544984e..670535a68d3b 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
>> @@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev)
>>       nouveau_display(dev)->fini = nv04_display_fini;
>>
>>       /* Pre-nv50 doesn't support atomic, so don't expose the ioctls */
>> -     dev->driver->driver_features &= ~DRIVER_ATOMIC;
>> +     dev->driver_features &= ~DRIVER_ATOMIC;
>>
>>       nouveau_hw_save_vga_fonts(dev, 1);
>>
>



-- 
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 1/2] drm/nouveau: Disable atomic support on a per-device basis

[Lyude Paul]

On Fri, 2018-09-14 at 10:11 +0200, Daniel Vetter wrote:
> On Thu, Sep 13, 2018 at 11:02 PM, Lyude Paul <lyude@redhat.com> wrote:
> > Hm, one nitpick here. Since /sys/kernel/debug/dri/*/state creation depends
> > on
> > the driver supporting atomic, maybe it would be good to make it so that we
> > set
> > DRIVER_ATOMIC in the driver_stub structure, then disable it per-device
> > depending
> > on the nouveau_atomic setting + hw support. That way we can always have the
> > state debugfs file while maintaining nouveau's current behavior with
> > exposing
> > atomic ioctls. Assuming that wouldn't have any unintended side-effects of
> > course
> 
> dri/*/state only works with atomic drivers. There's no explicit state
> with legacy drivers at all, it's all just implicit in hw and some
> random driver structures.
> 
> We should make sure though that the debugfs stuff looks at
> drm_drv_uses_atomic_modsetting(), and not DRIVER_ATOMIC. Former is
> about the internals (i915 is internally atomic everywhere), latter
> about the uapi (some old platforms aren't properly validated for full
> atomic features, hence why it's disabled).
> -Daniel
Makes sense, it seems doing that results in exactly what I wanted with nouveau!
As for this patch:

Reviewed-by: Lyude Paul <lyude@redhat.com>

> 
> > On Thu, 2018-09-13 at 19:31 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We now have per-device driver_features, so let's use that
> > > to disable atomic only for pre-nv50.
> > > 
> > > Cc: Ben Skeggs <bskeggs@redhat.com>
> > > Cc: Lyude Paul <lyude@redhat.com>
> > > Cc: nouveau@lists.freedesktop.org
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > > b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > > index 70dce544984e..670535a68d3b 100644
> > > --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > > +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > > @@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev)
> > >       nouveau_display(dev)->fini = nv04_display_fini;
> > > 
> > >       /* Pre-nv50 doesn't support atomic, so don't expose the ioctls */
> > > -     dev->driver->driver_features &= ~DRIVER_ATOMIC;
> > > +     dev->driver_features &= ~DRIVER_ATOMIC;
> > > 
> > >       nouveau_hw_save_vga_fonts(dev, 1);
> > > 
> 
> 
> 

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

Re: [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 05:02:05PM -0400, Lyude Paul wrote:
> Hm, one nitpick here. Since /sys/kernel/debug/dri/*/state creation depends on
> the driver supporting atomic, maybe it would be good to make it so that we set
> DRIVER_ATOMIC in the driver_stub structure, then disable it per-device depending
> on the nouveau_atomic setting + hw support. That way we can always have the
> state debugfs file while maintaining nouveau's current behavior with exposing
> atomic ioctls. Assuming that wouldn't have any unintended side-effects of course

I'm not sure why there are three driver structs in nouveau.
Maybe someone can just nuke two of them?

> 
> On Thu, 2018-09-13 at 19:31 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We now have per-device driver_features, so let's use that
> > to disable atomic only for pre-nv50.
> > 
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: Lyude Paul <lyude@redhat.com>
> > Cc: nouveau@lists.freedesktop.org
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > index 70dce544984e..670535a68d3b 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> > @@ -56,7 +56,7 @@ nv04_display_create(struct drm_device *dev)
> >  	nouveau_display(dev)->fini = nv04_display_fini;
> >  
> >  	/* Pre-nv50 doesn't support atomic, so don't expose the ioctls */
> > -	dev->driver->driver_features &= ~DRIVER_ATOMIC;
> > +	dev->driver_features &= ~DRIVER_ATOMIC;
> >  
> >  	nouveau_hw_save_vga_fonts(dev, 1);
> >  

-- 
Ville Syrjälä
Intel
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH 1/2] drm/nouveau: Disable atomic support on a per-device basis

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4119 bytes --]

Hi Ville,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc3 next-20180913]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ville-Syrjala/drm-nouveau-Disable-atomic-support-on-a-per-device-basis/20180914-111059
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/nouveau/dispnv04/disp.c: In function 'nv04_display_create':
>> drivers/gpu/drm/nouveau/dispnv04/disp.c:59:5: error: 'struct drm_device' has no member named 'driver_features'
     dev->driver_features &= ~DRIVER_ATOMIC;
        ^~

vim +59 drivers/gpu/drm/nouveau/dispnv04/disp.c

    33	
    34	int
    35	nv04_display_create(struct drm_device *dev)
    36	{
    37		struct nouveau_drm *drm = nouveau_drm(dev);
    38		struct nvkm_i2c *i2c = nvxx_i2c(&drm->client.device);
    39		struct dcb_table *dcb = &drm->vbios.dcb;
    40		struct drm_connector *connector, *ct;
    41		struct drm_encoder *encoder;
    42		struct nouveau_encoder *nv_encoder;
    43		struct nouveau_crtc *crtc;
    44		struct nv04_display *disp;
    45		int i, ret;
    46	
    47		disp = kzalloc(sizeof(*disp), GFP_KERNEL);
    48		if (!disp)
    49			return -ENOMEM;
    50	
    51		nvif_object_map(&drm->client.device.object, NULL, 0);
    52	
    53		nouveau_display(dev)->priv = disp;
    54		nouveau_display(dev)->dtor = nv04_display_destroy;
    55		nouveau_display(dev)->init = nv04_display_init;
    56		nouveau_display(dev)->fini = nv04_display_fini;
    57	
    58		/* Pre-nv50 doesn't support atomic, so don't expose the ioctls */
  > 59		dev->driver_features &= ~DRIVER_ATOMIC;
    60	
    61		nouveau_hw_save_vga_fonts(dev, 1);
    62	
    63		nv04_crtc_create(dev, 0);
    64		if (nv_two_heads(dev))
    65			nv04_crtc_create(dev, 1);
    66	
    67		for (i = 0; i < dcb->entries; i++) {
    68			struct dcb_output *dcbent = &dcb->entry[i];
    69	
    70			connector = nouveau_connector_create(dev, dcbent->connector);
    71			if (IS_ERR(connector))
    72				continue;
    73	
    74			switch (dcbent->type) {
    75			case DCB_OUTPUT_ANALOG:
    76				ret = nv04_dac_create(connector, dcbent);
    77				break;
    78			case DCB_OUTPUT_LVDS:
    79			case DCB_OUTPUT_TMDS:
    80				ret = nv04_dfp_create(connector, dcbent);
    81				break;
    82			case DCB_OUTPUT_TV:
    83				if (dcbent->location == DCB_LOC_ON_CHIP)
    84					ret = nv17_tv_create(connector, dcbent);
    85				else
    86					ret = nv04_tv_create(connector, dcbent);
    87				break;
    88			default:
    89				NV_WARN(drm, "DCB type %d not known\n", dcbent->type);
    90				continue;
    91			}
    92	
    93			if (ret)
    94				continue;
    95		}
    96	
    97		list_for_each_entry_safe(connector, ct,
    98					 &dev->mode_config.connector_list, head) {
    99			if (!connector->encoder_ids[0]) {
   100				NV_WARN(drm, "%s has no encoders, removing\n",
   101					connector->name);
   102				connector->funcs->destroy(connector);
   103			}
   104		}
   105	
   106		list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
   107			struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
   108			struct nvkm_i2c_bus *bus =
   109				nvkm_i2c_bus_find(i2c, nv_encoder->dcb->i2c_index);
   110			nv_encoder->i2c = bus ? &bus->i2c : NULL;
   111		}
   112	
   113		/* Save previous state */
   114		list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
   115			crtc->save(&crtc->base);
   116	
   117		list_for_each_entry(nv_encoder, &dev->mode_config.encoder_list, base.base.head)
   118			nv_encoder->enc_save(&nv_encoder->base.base);
   119	
   120		nouveau_overlay_init(dev);
   121	
   122		return 0;
   123	}
   124	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65150 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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