[Intel-gfx] [PATCH 3/3] drm: Store the calculated vrefresh in the user mode

[Ville Syrjälä]

On Tue, Mar 13, 2018 at 08:04:03PM +0100, Maarten Lankhorst wrote:
> Op 13-03-18 om 16:07 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >
> > Ignore the vrefresh in the mode the user passed in and instead
> > calculate the value based on the actual timings. This way we can
> > actually trust mode->vrefresh to some degree.
> >
> > Or should we compare the user's idea of vrefresh with the one
> > we get from the timings and return an error if they differ? We
> > can't really be sure what the user is asking in that case.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_modes.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index f6b7c0e36a1a..021526ec6fa0 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1609,7 +1609,11 @@ int drm_mode_convert_umode(struct drm_device *dev,
> >  	out->vsync_end = in->vsync_end;
> >  	out->vtotal = in->vtotal;
> >  	out->vscan = in->vscan;
> > -	out->vrefresh = in->vrefresh;
> > +	 /*
> > +	  * Ignore what the user is saying here and instead
> > +	  * calculate vrefresh based on the actual timings.
> > +	  */
> > +	out->vrefresh = 0;
> >  	out->flags = in->flags;
> >  	out->type = in->type;
> >  	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
> > @@ -1619,6 +1623,8 @@ int drm_mode_convert_umode(struct drm_device *dev,
> >  	if (out->status != MODE_OK)
> >  		return -EINVAL;
> >  
> > +	out->vrefresh = drm_mode_vrefresh(out);
> > +
> >  	drm_mode_set_crtcinfo(out, CRTC_INTERLACE_HALVE_V);
> >  
> >  	return 0;
> 
> Could we also get this with dim fixes tags dc911f5bd8aa, so we can backport the alt mode handling patch?

Do we want/need to backport it actually?

> 
> And update the documentation about vrefresh, that you can retrieve it with drm_mode_vrefresh?

The whole "return cached value if present, otherwise calculate but don't
update the cached value" apporach always seemed off to me. Might be a
good idea to change it somehow. Maybe just always calculate it, or do
the cached value updates in sensible places so that you only have to
calculate once. But I haven't actually checked how much work that
would entail.

-- 
Ville Syrjälä
Intel OTC