Op 13-03-18 om 15:27 schreef Ville Syrjälä: > 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 <[email protected]> >> Fixes: dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP if >> available.") >> Cc: David Weinehall <[email protected]> >> Cc: Rodrigo Vivi <[email protected]> >> Cc: Paulo Zanoni <[email protected]> >> Cc: Jani Nikula <[email protected]> >> Cc: Chris Wilson <[email protected]> >> Cc: Jim Bride <[email protected]> >> Cc: Joonas Lahtinen <[email protected]> >> Cc: <[email protected]> # 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. There's no guarantee vrefresh is set to something sane, afaik. I don't think we can use it. If it was always set, then maybe. But I think nothing prevents it from being garbage.
~Maarten _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
