On Mon, Oct 09, 2023 at 10:58:22AM +0000, Kandpal, Suraj wrote:
>
>
> > -----Original Message-----
> > From: Jani Nikula <[email protected]>
> > Sent: Monday, October 9, 2023 4:07 PM
> > To: Kandpal, Suraj <[email protected]>;
> > [email protected]
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Free crtc_state in
> > verify_crtc_state
> >
> > On Mon, 09 Oct 2023, Suraj Kandpal <[email protected]> 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ä <[email protected]>
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 <[email protected]>
> > > ---
> > > 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