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

[Kandpal, Suraj]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Monday, October 9, 2023 6:08 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>; intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Free crtc_state in
> verify_crtc_state
> 
> On Mon, Oct 09, 2023 at 10:58:22AM +0000, Kandpal, Suraj wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jani Nikula <jani.nikula at linux.intel.com>
> > > Sent: Monday, October 9, 2023 4:07 PM
> > > To: Kandpal, Suraj <suraj.kandpal at intel.com>;
> > > intel-gfx at 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 at 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 at 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...

Sure will also float patches fixing the above mentioned issue

Regards,
Suraj Kandpal
> 
> > Regards,
> > Suraj Kandpal
> > >
> > > >
> > > > Signed-off-by: Suraj Kandpal <suraj.kandpal at 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