ACK: [SRU][Artful][PATCH 1/1] UBUNTU: SAUCE: Revert "drm/i915/edp: Allow alternate fixed mode for eDP if available."
Kleber Souza
kleber.souza at canonical.com
Tue Jun 5 18:30:32 UTC 2018
On 06/01/18 08:40, Joseph Salisbury wrote:
> From: Jani Nikula <jani.nikula at intel.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1773520
>
> This reverts commit dc911f5bd8aacfcf8aabd5c26c88e04c837a938e.
>
> Per the report, no matter what display mode you select with xrandr, the
> i915 driver will always select the alternate fixed mode. For the
> reporter this means that the display will always run at 40Hz which is
> quite annoying. This may be due to the mode comparison.
>
> But there are some other potential issues. The choice of alt_fixed_mode
> seems dubious. It's the first non-preferred mode, but there are no
> guarantees that the only difference would be refresh rate. Similarly,
> there may be more than one preferred mode in the probed modes list, and
> the commit changes the preferred mode selection to choose the last one
> on the list instead of the first.
>
> (Note that the probed modes list is the raw, unfiltered, unsorted list
> of modes from drm_add_edid_modes(), not the pretty result after a
> drm_helper_probe_single_connector_modes() call.)
>
> Finally, we already have eerily similar code in place to find the
> downclock mode for DRRS that seems like could be reused here.
>
> Back to the drawing board.
>
> Note: This is a hand-crafted revert due to conflicts. If it fails to
> backport, please just try reverting the original commit directly.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105469
> Reported-by: Rune Petersen <rune at megahurts.dk>
> Reported-by: Mark Spencer <n7u4722r35 at ynzlx.anonbox.net>
> Fixes: dc911f5bd8aa ("drm/i915/edp: Allow alternate fixed mode for eDP if available.")
> Cc: Clint Taylor <clinton.a.taylor at intel.com>
> Cc: David Weinehall <david.weinehall at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jim Bride <jim.bride at linux.intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Cc: <stable at vger.kernel.org> # v4.14+
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20180516080110.22770-1-jani.nikula@intel.com
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 38 +++++---------------------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 --
> drivers/gpu/drm/i915/intel_dsi.c | 2 +-
> drivers/gpu/drm/i915/intel_dvo.c | 2 +-
> drivers/gpu/drm/i915/intel_lvds.c | 3 +--
> drivers/gpu/drm/i915/intel_panel.c | 6 ------
> 6 files changed, 8 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 64e59e2..43c8f92 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1606,23 +1606,6 @@ 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)
> -{
> - bool bres = false;
> -
> - 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;
> -}
> -
> bool
> intel_dp_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config,
> @@ -1669,16 +1652,8 @@ 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;
> -
> - if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
> - panel_mode = intel_connector->panel.fixed_mode;
> -
> - drm_mode_debug_printmodeline(panel_mode);
> -
> - intel_fixed_panel_mode(panel_mode, adjusted_mode);
> + intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> + adjusted_mode);
>
> if (INTEL_GEN(dev_priv) >= 9) {
> int ret;
> @@ -5749,7 +5724,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> struct drm_device *dev = intel_encoder->base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_display_mode *fixed_mode = NULL;
> - struct drm_display_mode *alt_fixed_mode = NULL;
> struct drm_display_mode *downclock_mode = NULL;
> bool has_dpcd;
> struct drm_display_mode *scan;
> @@ -5805,14 +5779,13 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> }
> intel_connector->edid = edid;
>
> - /* prefer fixed mode from EDID if available, save an alt mode also */
> + /* prefer fixed mode from EDID if available */
> list_for_each_entry(scan, &connector->probed_modes, head) {
> if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
> fixed_mode = drm_mode_duplicate(dev, scan);
> downclock_mode = intel_dp_drrs_init(
> intel_connector, fixed_mode);
> - } else if (!alt_fixed_mode) {
> - alt_fixed_mode = drm_mode_duplicate(dev, scan);
> + break;
> }
> }
>
> @@ -5849,8 +5822,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> pipe_name(pipe));
> }
>
> - intel_panel_init(&intel_connector->panel, fixed_mode, alt_fixed_mode,
> - downclock_mode);
> + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> intel_connector->panel.backlight.power = intel_edp_backlight_power;
> intel_panel_setup_backlight(connector, pipe);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b5a0cf1..468c011 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -265,7 +265,6 @@ struct intel_encoder {
>
> struct intel_panel {
> struct drm_display_mode *fixed_mode;
> - struct drm_display_mode *alt_fixed_mode;
> struct drm_display_mode *downclock_mode;
>
> /* backlight */
> @@ -1697,7 +1696,6 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
> /* intel_panel.c */
> int intel_panel_init(struct intel_panel *panel,
> struct drm_display_mode *fixed_mode,
> - struct drm_display_mode *alt_fixed_mode,
> struct drm_display_mode *downclock_mode);
> void intel_panel_fini(struct intel_panel *panel);
> void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 0fdd4f1..4b8f6e0 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1840,7 +1840,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> connector->display_info.width_mm = fixed_mode->width_mm;
> connector->display_info.height_mm = fixed_mode->height_mm;
>
> - intel_panel_init(&intel_connector->panel, fixed_mode, NULL, NULL);
> + intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> intel_panel_setup_backlight(connector, INVALID_PIPE);
>
> intel_dsi_add_properties(intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 39fd4f3..c1544a5 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -554,7 +554,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
> */
> intel_panel_init(&intel_connector->panel,
> intel_dvo_get_current_mode(connector),
> - NULL, NULL);
> + NULL);
> intel_dvo->panel_wants_dither = true;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index a995ccb..6fe5d7c 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -1140,8 +1140,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
> out:
> mutex_unlock(&dev->mode_config.mutex);
>
> - intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
> - downclock_mode);
> + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> intel_panel_setup_backlight(connector, INVALID_PIPE);
>
> lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index cf395cd..593349b 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1919,13 +1919,11 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>
> int intel_panel_init(struct intel_panel *panel,
> struct drm_display_mode *fixed_mode,
> - struct drm_display_mode *alt_fixed_mode,
> struct drm_display_mode *downclock_mode)
> {
> intel_panel_init_backlight_funcs(panel);
>
> panel->fixed_mode = fixed_mode;
> - panel->alt_fixed_mode = alt_fixed_mode;
> panel->downclock_mode = downclock_mode;
>
> return 0;
> @@ -1939,10 +1937,6 @@ void intel_panel_fini(struct intel_panel *panel)
> if (panel->fixed_mode)
> drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
>
> - if (panel->alt_fixed_mode)
> - drm_mode_destroy(intel_connector->base.dev,
> - panel->alt_fixed_mode);
> -
> if (panel->downclock_mode)
> drm_mode_destroy(intel_connector->base.dev,
> panel->downclock_mode);
>
More information about the kernel-team
mailing list