Re: [Intel-gfx] [PATCH] drm/i915/display: Free crtc_state in verify_crtc_state

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Oct 09, 2023 at 10:58:22AM +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Jani Nikula <jani.nikula@linux.intel.com>
> > Sent: Monday, October 9, 2023 4:07 PM
> > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Free crtc_state in
> > verify_crtc_state
> > 
> > On Mon, 09 Oct 2023, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
> > > Free hw_crtc_state in verify_crtc_state after we are done using this
> > > or else it's just a resource leak.
> > 
> > Fixes: ?
> 
> Cant really trace when this was introduced as this function was split from
> Intel_display.c to intel_modeset_verify.c and then this is the last commit that introduces it
> df17ff62b626 drm/i915/display: split out modeset verification code
> 
> Should I use this commit in the fixes tag?
>

Mea culpa
Fixes: 2745bdda2095 ("drm/i915: Stop clobbering old crtc state during state check")
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

BTW looks like intel_encoder_current_mode() can also leak some stuff
on account of using kfree() instead of intel_crtc_destroy_state().
Can you cook up a patch to fix that one as well?

And while vlv_force_pll_on() isn't actually leaking, it'd probably
a good idea to switch over to intel_crtc_destroy_state() there as well...

> Regards,
> Suraj Kandpal
> > 
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_modeset_verify.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > > b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > > index 303eb68fec11..5e1c2c780412 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_modeset_verify.c
> > > @@ -214,7 +214,7 @@ verify_crtc_state(struct intel_atomic_state *state,
> > >  	}
> > >
> > >  	if (!sw_crtc_state->hw.active)
> > > -		return;
> > > +		goto destroy_state;
> > >
> > >  	intel_pipe_config_sanity_check(hw_crtc_state);
> > >
> > > @@ -224,6 +224,9 @@ verify_crtc_state(struct intel_atomic_state *state,
> > >  		intel_crtc_state_dump(hw_crtc_state, NULL, "hw state");
> > >  		intel_crtc_state_dump(sw_crtc_state, NULL, "sw state");
> > >  	}
> > > +
> > > +destroy_state:
> > > +	intel_crtc_destroy_state(&crtc->base, &hw_crtc_state->uapi);
> > >  }
> > >
> > >  void intel_modeset_verify_crtc(struct intel_atomic_state *state,
> > 
> > --
> > Jani Nikula, Intel

-- 
Ville Syrjälä
Intel