On Thu, 2023-09-28 at 10:43 +0300, Jani Nikula wrote: > On Thu, 28 Sep 2023, Jouni Högander <jouni.hogander@xxxxxxxxx> 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@xxxxxxxxx> > > > > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx> > > --- > > 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); >
