[Intel-gfx] [PATCH v2] drm/i915: Warn on if set frontbuffer return value is not NULL on release
Hogander, Jouni
jouni.hogander at intel.com
Thu Sep 28 08:31:02 UTC 2023
On Thu, 2023-09-28 at 10:43 +0300, Jani Nikula wrote:
> On Thu, 28 Sep 2023, Jouni Högander <jouni.hogander at intel.com> wrote:
> > i915_gem_object_set_frontbuffer returns set frontbuffer pointer.
> > When we
> > are releasing frontbuffer we are clearing the pointer from the
> > object. Warn
> > on if return value is not null.
> >
> > v2: Instead of ignoring do drm_WARN_ON
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >
> > Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_frontbuffer.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > index d5540c739404..95319301cf2b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > @@ -259,7 +259,8 @@ static void frontbuffer_release(struct kref
> > *ref)
> >
> > i915_ggtt_clear_scanout(obj);
> >
> > - i915_gem_object_set_frontbuffer(obj, NULL);
> > + drm_WARN_ON(&intel_bo_to_i915(obj)->drm,
> > + i915_gem_object_set_frontbuffer(obj, NULL) !=
> > NULL);
>
> I'm in the camp that says drm_WARN_ON() and friends should not
> contain
> any functional actions, and it should just be for checks. I.e. if you
> removed all the warns, things would still work, and lines that do
> stuff
> stand out and aren't hidden in WARN parameters.
>
> Like so:
>
> ret = i915_gem_object_set_frontbuffer(obj, NULL);
> drm_WARN_ON(&intel_bo_to_i915(obj)->drm, ret);
Thank you Jani for bringing up this aspect. Just sent a new version of
this patch. Please check.
BR,
Jouni Högander
>
> BR,
> Jani.
>
> > spin_unlock(&intel_bo_to_i915(obj)-
> > >display.fb_tracking.lock);
> >
> > i915_active_fini(&front->write);
>
More information about the Intel-gfx
mailing list