diff mbox

[10/37] drm/nouveau: Fix crtc->primary->fb vs. drm_fb fail

Message ID 1479498793-31021-11-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Nov. 18, 2016, 7:52 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

So it looks like the code is trying to pick between the passed in fb and
crtc->primary->fb based on that funky 'bool atomic'. But later it will
mix uses of both drm_fb (which was picked by the aforementioned logic)
and crtc->primary->fb. So looks like a bug to me. Let's make it use
drm_fb only.

Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Nov. 30, 2016, 2:56 p.m. UTC | #1 
On Fri, Nov 18, 2016 at 09:52:46PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> So it looks like the code is trying to pick between the passed in fb and
> crtc->primary->fb based on that funky 'bool atomic'. But later it will
> mix uses of both drm_fb (which was picked by the aforementioned logic)
> and crtc->primary->fb. So looks like a bug to me. Let's make it use
> drm_fb only.
> 
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Oh dear, set_base_atomic indeed expects that the passed-in fb is used, and
that drivers do _not_ update crtc->primary->fb. And drm core expects them
to do that, kinda, or at least allows it. I guess time to nuke all that
kgdb stuff as dead code and unused since obviously all broken.

But that's another story, this here seems to make some sense.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 59d1d1c5de5f..7c6c66f177df 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -854,9 +854,9 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
>  
>  	/* Update the framebuffer format. */
>  	regp->CRTC[NV_CIO_CRE_PIXEL_INDEX] &= ~3;
> -	regp->CRTC[NV_CIO_CRE_PIXEL_INDEX] |= (crtc->primary->fb->depth + 1) / 8;
> +	regp->CRTC[NV_CIO_CRE_PIXEL_INDEX] |= (drm_fb->depth + 1) / 8;
>  	regp->ramdac_gen_ctrl &= ~NV_PRAMDAC_GENERAL_CONTROL_ALT_MODE_SEL;
> -	if (crtc->primary->fb->depth == 16)
> +	if (drm_fb->depth == 16)
>  		regp->ramdac_gen_ctrl |= NV_PRAMDAC_GENERAL_CONTROL_ALT_MODE_SEL;
>  	crtc_wr_cio_state(crtc, regp, NV_CIO_CRE_PIXEL_INDEX);
>  	NVWriteRAMDAC(dev, nv_crtc->index, NV_PRAMDAC_GENERAL_CONTROL,
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 59d1d1c5de5f..7c6c66f177df 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -854,9 +854,9 @@  nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
 
 	/* Update the framebuffer format. */
 	regp->CRTC[NV_CIO_CRE_PIXEL_INDEX] &= ~3;
-	regp->CRTC[NV_CIO_CRE_PIXEL_INDEX] |= (crtc->primary->fb->depth + 1) / 8;
+	regp->CRTC[NV_CIO_CRE_PIXEL_INDEX] |= (drm_fb->depth + 1) / 8;
 	regp->ramdac_gen_ctrl &= ~NV_PRAMDAC_GENERAL_CONTROL_ALT_MODE_SEL;
-	if (crtc->primary->fb->depth == 16)
+	if (drm_fb->depth == 16)
 		regp->ramdac_gen_ctrl |= NV_PRAMDAC_GENERAL_CONTROL_ALT_MODE_SEL;
 	crtc_wr_cio_state(crtc, regp, NV_CIO_CRE_PIXEL_INDEX);
 	NVWriteRAMDAC(dev, nv_crtc->index, NV_PRAMDAC_GENERAL_CONTROL,