Re: [PATCH v2 2/2] drm/i915/psr: check drm_mode_vrefresh return value

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Fri, 2025-08-22 at 17:12 +0300, Ville Syrjälä wrote:
> On Fri, Aug 22, 2025 at 05:07:18AM +0000, Hogander, Jouni wrote:
> > On Thu, 2025-08-21 at 16:10 +0300, Ville Syrjälä wrote:
> > > On Thu, Aug 21, 2025 at 07:59:18AM +0300, Jouni Högander wrote:
> > > > Check drm_mode_vrefresh return value sanity before using it in
> > > > intel_get_frame_time_us.
> > > > 
> > > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 5addde63168e..8cc2314fac6f 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -1116,11 +1116,16 @@ transcoder_has_psr2(struct
> > > > intel_display
> > > > *display, enum transcoder cpu_transcode
> > > >  
> > > >  static u32 intel_get_frame_time_us(const struct
> > > > intel_crtc_state
> > > > *crtc_state)
> > > >  {
> > > > +	int vrefresh;
> > > > +
> > > >  	if (!crtc_state->hw.active)
> > > >  		return 0;
> > > >  
> > > > -	return DIV_ROUND_UP(1000 * 1000,
> > > > -			    drm_mode_vrefresh(&crtc_state-
> > > > > hw.adjusted_mode));
> > > > +	vrefresh = drm_mode_vrefresh(&crtc_state-
> > > > > hw.adjusted_mode);
> > > > +	if (vrefresh <= 0)
> > > 
> > > How would that happen?
> > 
> > There are some sanity checks in drm_mode_vrefresh returning 0. Do
> > you
> > think we should just rely on that we would not end up here with
> > such
> > broken drm mode data?
> 
> If you find some way to get this far with a bogus mode then we must
> have a gap in mode valiation somewhere. drm_mode_vrefresh() itself
> must tolerate some amount of nonsense as it may be fed with garbage
> from userspace.
> 

Ok, I will drop this patch. I will merge the first patch in the set.

BR,

Jouni Högander