On Tue, Mar 13, 2018 at 02:52:35PM +0100, Maarten Lankhorst wrote:
> On fi-cnl-y3 we have 2 modes that differ only by crtc_clock. This means
> that if we request the normal mode, we automatically get the downclocked
> mode.
> 
> This can be seen during boot:
> [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:101:eDP-1] probed 
> modes :
> [drm:drm_mode_debug_printmodeline] Modeline 102:"3840x2160" 60 533250 3840 
> 3888 3920 4000 2160 2163 2168 2222 0x48 0xa
> [drm:drm_mode_debug_printmodeline] Modeline 103:"3840x2160" 48 426600 3840 
> 3888 3920 4000 2160 2163 2168 2222 0x40 0xa
> ...
> [drm:intel_dump_pipe_config [i915]] [CRTC:51:pipe A][modeset]
> [drm:intel_dump_pipe_config [i915]] output_types: EDP (0x100)
> [drm:intel_dump_pipe_config [i915]] cpu_transcoder: EDP, pipe bpp: 24, 
> dithering: 0
> [drm:intel_dump_pipe_config [i915]] dp m_n: lanes: 4; gmch_m: 4970250, 
> gmch_n: 8388608, link_m: 828375, link_n: 1048576, tu: 64
> [drm:intel_dump_pipe_config [i915]] dp m2_n2: lanes: 4; gmch_m: 4970250, 
> gmch_n: 8388608, link_m: 828375, link_n: 1048576, tu: 64
> [drm:intel_dump_pipe_config [i915]] audio: 0, infoframes: 0
> [drm:intel_dump_pipe_config [i915]] requested mode:
> [drm:drm_mode_debug_printmodeline] Modeline 0:"3840x2160" 60 533250 3840 3888 
> 3920 4000 2160 2163 2168 2222 0x48 0xa
> [drm:intel_dump_pipe_config [i915]] adjusted mode:
> [drm:drm_mode_debug_printmodeline] Modeline 0:"3840x2160" 48 426600 3840 3888 
> 3920 4000 2160 2163 2168 2222 0x40 0xa
> 
> Add a intel_edp_choose_mode helper that picks the mode.
> 
> If there is no alt mode, it always picks the default mode.
> If the alt mode is identical to the default mode, it picks the mode
> based on what mode is closest to the request clock.
> If the alt mode differs, it only picks the alt mode on exact match.
> 
> Testcase: kms_panel_fitting.atomic-fastset
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Fixes: dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP if 
> available.")
> Cc: David Weinehall <david.weineh...@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> Cc: Jani Nikula <jani.nik...@intel.com>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Jim Bride <jim.br...@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: <sta...@vger.kernel.org> # v4.14+
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469
> References: https://bugs.freedesktop.org/show_bug.cgi?id=105456
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 39 +++++++++++++++++++++------------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9a4a51e79fa1..075981842a7f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1671,21 +1671,25 @@ static int intel_dp_compute_bpp(struct intel_dp 
> *intel_dp,
>       return bpp;
>  }
>  
> -static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
> -                                    struct drm_display_mode *m2)
> +static const struct drm_display_mode *
> +intel_edp_choose_mode(const struct drm_display_mode *req,
> +                   const struct drm_display_mode *def,
> +                   const struct drm_display_mode *alt)
>  {
> -     bool bres = false;
> +     if (!alt)
> +             return def;
>  
> -     if (m1 && m2)
> -             bres = (m1->hdisplay == m2->hdisplay &&
> -                     m1->hsync_start == m2->hsync_start &&
> -                     m1->hsync_end == m2->hsync_end &&
> -                     m1->htotal == m2->htotal &&
> -                     m1->vdisplay == m2->vdisplay &&
> -                     m1->vsync_start == m2->vsync_start &&
> -                     m1->vsync_end == m2->vsync_end &&
> -                     m1->vtotal == m2->vtotal);
> -     return bres;
> +     if (drm_mode_equal_no_clocks(def, alt)) {
> +             int clock;
> +
> +             if (!drm_mode_equal_no_clocks(req, def))
> +                     return def;
> +
> +             clock = def->clock / 2 + alt->clock / 2;
> +
> +             return req->clock > clock ? def : alt;
> +     } else
> +             return drm_mode_equal_no_clocks(req, alt) ? alt : def;

Seems a bit convoluted to me.

return user_mode->vrefresh > alt_mode->vrefresh ?
        fixed_mode : alt_mode;

maybe?

Though we don't seem to try very hard to make vrefresh accurate so not
100% this is going to work perfectly.

I guess we could go with the "pick the closest" vrefresh approach
instead. I'm thinking we should actually be rejecting the user mode
entirely if it isn't closeto either refresh rate.

>  }
>  
>  bool
> @@ -1734,12 +1738,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>               pipe_config->has_audio = intel_conn_state->force_audio == 
> HDMI_AUDIO_ON;
>  
>       if (intel_dp_is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> -             struct drm_display_mode *panel_mode =
> -                     intel_connector->panel.alt_fixed_mode;
> -             struct drm_display_mode *req_mode = &pipe_config->base.mode;
> +             const struct drm_display_mode *panel_mode;
>  
> -             if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> -                     panel_mode = intel_connector->panel.fixed_mode;
> +             panel_mode = intel_edp_choose_mode(&pipe_config->base.mode,
> +                                                
> intel_connector->panel.fixed_mode,
> +                                                
> intel_connector->panel.alt_fixed_mode);
>  
>               drm_mode_debug_printmodeline(panel_mode);
>  
> -- 
> 2.16.2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to