Quoting Jani Nikula (2023-10-02 04:08:40-03:00) >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! Yep... Not sure, but maybe this could be improved to somehow try to capture only functions that have some side-effect (e.g., register writes or modification to driver data). -- Gustavo Sousa > >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
