Re: [PATCH] drm/i915: Fix frontbuffer false positve.

Daniel Vetter
2015-02-24T20:36:49Z

On Tue, Feb 24, 2015 at 10:44:19AM -0800, Matt Roper wrote:
> On Tue, Feb 24, 2015 at 10:36:32AM -0800, Rodrigo Vivi wrote:
> > Atomic bits needs to be set when cursor check function is returning 0
> > and intel_crtc is active.
> >
> > v2: When putting more debug prints I notice the solution was simpler
> > than I thought. AMS design is solid, just this return was wrong.
> > Sorry for the noise.
> >
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8ccf033..5fb83ba 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12198,7 +12198,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
> > }
> >
> > if (fb == crtc->cursor->fb)
> > - return 0;
> > + goto finish;

Shouldn't we just delete this check entirely? gcc will do it anyway for
us. When you do that you can also append the history I've dug out for
this.

The original regression seems to have been introduced in the original
check/commit split:

commit 757f9a3e5b8a812af0c213099a5b31cb423f4d3c
Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Date: Wed Sep 24 14:20:24 2014 -0300

drm/i915: move check of intel_crtc_cursor_set_obj() out

Which already cause other trouble, resulting in the check getting moved in

commit e391ea882b1a04fb3f559287ac694652a3cd9da9
Author: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Date: Wed Sep 24 14:20:25 2014 -0300

drm/i915: Fix not checking cursor and object sizes

The frontbuffer tracking itself only was broken when we shifted it into
the check/commit logic with:

commit 32b7eeec4d1e861230b09d437e95d76c86ff4a68
Author: Matt Roper <matthew.d.roper@intel.com>
Date: Wed Dec 24 07:59:06 2014 -0800

drm/i915: Refactor work that can sleep out of commit (v7)

Cheers, Daniel
> >
> > if (fb->modifier[0] != DRM_FORMAT_MOD_NONE) {
> > DRM_DEBUG_KMS("cursor cannot be tiled\n");
> > --
> > 2.1.0
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch