Re: [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ä 
> >
> > 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ä 
> > ---
> >  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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Maarten Lankhorst]

Op 13-03-18 om 16:07 schreef Ville Syrjala:
> From: Ville Syrjälä 
>
> 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ä 
> ---
>  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?

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

~Maarten

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx