| Message ID | 1499811596-14634-4-git-send-email-jim.bride@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Jim Bride (2017-07-11 23:19:55) > @@ -5869,13 +5891,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > } > intel_connector->edid = edid; > > - /* prefer fixed mode from EDID if available */ > + /* prefer fixed mode from EDID if available, save an alt mode also */ > 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); > - break; > + } else { > + alt_fixed_mode = drm_mode_duplicate(dev, scan); > } > } Save one, leak the rest? > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 96c2cbd..4e7ba93 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1919,11 +1919,13 @@ 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; Ah, at least it is consistent; they all get leaked. :) In general though, it looks like for edp you want to scan connector->probed_modes at config time looking for an exact match in preference to using the scaled mode. -Chris
On Wed, Jul 12, 2017 at 12:27:33AM +0100, Chris Wilson wrote: > Quoting Jim Bride (2017-07-11 23:19:55) > > @@ -5869,13 +5891,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > > } > > intel_connector->edid = edid; > > > > - /* prefer fixed mode from EDID if available */ > > + /* prefer fixed mode from EDID if available, save an alt mode also */ > > 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); > > - break; > > + } else { > > + alt_fixed_mode = drm_mode_duplicate(dev, scan); > > } > > } > > Save one, leak the rest? Good catch, I'll tweak the code so that we only duplicate once, although in practice (on the panels I have to test with) fixed panels support either one or two modes, with only the frame rate being different if two are supported. > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > > index 96c2cbd..4e7ba93 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -1919,11 +1919,13 @@ 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; > > Ah, at least it is consistent; they all get leaked. :) Yeah, I missed adding a destroy call in intel_panel_fini(). I'll fix that. Thanks for the review! Jim > In general though, it looks like for edp you want to scan > connector->probed_modes at config time looking for an exact match in > preference to using the scaled mode. > -Chris
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 69c8130c..ee2a3db 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1606,6 +1606,19 @@ 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) +{ + return (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); +} + bool intel_dp_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, @@ -1652,8 +1665,16 @@ intel_dp_compute_config(struct intel_encoder *encoder, pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON; if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) { - intel_fixed_panel_mode(intel_connector->panel.fixed_mode, - adjusted_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); if (INTEL_GEN(dev_priv) >= 9) { int ret; @@ -5814,6 +5835,7 @@ 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; @@ -5869,13 +5891,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, } intel_connector->edid = edid; - /* prefer fixed mode from EDID if available */ + /* prefer fixed mode from EDID if available, save an alt mode also */ 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); - break; + } else { + alt_fixed_mode = drm_mode_duplicate(dev, scan); } } @@ -5912,7 +5935,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, pipe_name(pipe)); } - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); + intel_panel_init(&intel_connector->panel, fixed_mode, alt_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 d17a324..0c14b05 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -265,6 +265,7 @@ 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 */ @@ -1696,6 +1697,7 @@ 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 50ec836..d3a6608 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1851,7 +1851,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); + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, 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 c1544a5..39fd4f3 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 6fe5d7c..a995ccb 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1140,7 +1140,8 @@ 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, downclock_mode); + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, + 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 96c2cbd..4e7ba93 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1919,11 +1919,13 @@ 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;
Some fixed resolution panels actually support more than one mode, with the only thing different being the refresh rate. Having this alternate mode available to us is desirable, because it allows us to test PSR on panels whose setup time at the preferred mode is too long. With this patch we allow the use of the alternate mode if it's available and it was specifically requested. v2: Rebase Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Jim Bride <jim.bride@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++++++++++++++++----- 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 | 2 ++ 6 files changed, 37 insertions(+), 8 deletions(-)