On Thu, Jun 28, 2018 at 10:43:01PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> We only ever drive the panel with the fixed mode, hence we don't
> want to advertize any modes that have a different vertical refresh rate.
>
> We tried to allow a second lower clocked mode to used for eDP but
> that was reverted in commit d93fa1b47b8f ("Revert "drm/i915/edp:
> Allow alternate fixed mode for eDP if available.""). To do that properly
> I think we should probably just keep all (or just some?) of the probed
> modes around (presumably all of them actually work if the panel
> advertizes them), and we'd just pick the closest match based on the
> user mode.
>
> For now we don't have any of that so we shouldn't lie to the user
> that they can drive the panel at different refresh rate. Note that
> we still don't reject any user mode with bad refresh rate. That's
> going to be step two.
>
> TODO: Should we allow for a larger error margin?
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Not sure the current vrefresh is any good, since it's HZ resolution only.
That's not precise enough for the video aficionados, who insist on that
entire 1001/1000 business.
I did look around a bit with a quick git grep, and most places only use
->vrefresh for debug printing. Then there's a few that use it as an index
(e.g. i915 DRRS code), and then there's the totally misguided code in
adv7511 which thinks vrefresh is in kHz.
There's also quite a pile of mode->vrefresh ? mode->vrefresh :
drm_mode_vrefresh(mode) around, which really doesn't inspire confidence
that we'll ever catch them all. It feels like those "recompute vrefresh"
things have been cargo-culted for ages.
I think we need to clean this up harder:
- switch everyone over to drm_mode_vrefresh()
- nuke drm_display_mode->vrefresh
- add a new drm_mode_vrefresh_mHz (for milli-Hz, i.e. fixed point math
ftw)
- use that new function in the mode filtering, with a bit of fudge that
keeps the 1001/1000 modes separate, but papers over rounding errors. I
think we could even do a drm_mode_compare_vrefresh for that, we're
probably not the only ones who'd like to have such a function.
Probably a bit more work to type (but cocci might help here), definitely
less painful to make sure it stays correct I think. Thoughts?
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 3 +++
> drivers/gpu/drm/i915/intel_dsi.c | 2 ++
> drivers/gpu/drm/i915/intel_dvo.c | 2 ++
> drivers/gpu/drm/i915/intel_lvds.c | 2 ++
> drivers/gpu/drm/i915/intel_sdvo.c | 2 ++
> 5 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 5be07e1d816d..22feb0b144f6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -446,6 +446,9 @@ intel_dp_mode_valid(struct drm_connector *connector,
> if (mode->vdisplay > fixed_mode->vdisplay)
> return MODE_PANEL;
>
> + if (mode->vrefresh != fixed_mode->vrefresh)
> + return MODE_PANEL;
> +
> target_clock = fixed_mode->clock;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 3b7acb5a70b3..3d0758cec85a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1277,6 +1277,8 @@ intel_dsi_mode_valid(struct drm_connector *connector,
> return MODE_PANEL;
> if (mode->vdisplay > fixed_mode->vdisplay)
> return MODE_PANEL;
> + if (mode->vrefresh != fixed_mode->vrefresh)
> + return MODE_PANEL;
> if (fixed_mode->clock > max_dotclk)
> return MODE_CLOCK_HIGH;
> }
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 4e142ff49708..078bb848a49a 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -225,6 +225,8 @@ intel_dvo_mode_valid(struct drm_connector *connector,
> return MODE_PANEL;
> if (mode->vdisplay > fixed_mode->vdisplay)
> return MODE_PANEL;
> + if (mode->vrefresh != fixed_mode->vrefresh)
> + return MODE_PANEL;
>
> target_clock = fixed_mode->clock;
> }
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index bb06744d28a4..d312a8911b89 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -384,6 +384,8 @@ intel_lvds_mode_valid(struct drm_connector *connector,
> return MODE_PANEL;
> if (mode->vdisplay > fixed_mode->vdisplay)
> return MODE_PANEL;
> + if (mode->vrefresh != fixed_mode->vrefresh)
> + return MODE_PANEL;
> if (fixed_mode->clock > max_pixclk)
> return MODE_CLOCK_HIGH;
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index cd5d5e800486..4be4bae51652 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1650,6 +1650,8 @@ intel_sdvo_mode_valid(struct drm_connector *connector,
> return MODE_PANEL;
>
> if (mode->vdisplay > fixed_mode->vdisplay)
> +
> + if (mode->vrefresh != fixed_mode->vrefresh)
> return MODE_PANEL;
> }
>
> --
> 2.16.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
