On Fri, 29 Sep 2023, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote: > Quoting Jani Nikula (2023-09-28 04:43:13-03:00) >>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. > > Good rationale. > > Here is a command for finding places where a fix might be applicable :-) > > spatch --very-quiet --sp 'drm_WARN_ON(..., <+... E(...) ...+>)' drivers/gpu/drm/i915 Good hints, but also a *lot* of false positives! BR, Jani. > > -- > Gustavo Sousa > >> >>Like so: >> >> ret = i915_gem_object_set_frontbuffer(obj, NULL); >> drm_WARN_ON(&intel_bo_to_i915(obj)->drm, ret); >> >>BR, >>Jani. >> >>> spin_unlock(&intel_bo_to_i915(obj)->display.fb_tracking.lock); >>> >>> i915_active_fini(&front->write); >> >>-- >>Jani Nikula, Intel -- Jani Nikula, Intel
