On Thu, Aug 10, 2017 at 04:13:44PM +0300, Jani Nikula wrote: > On Wed, 09 Aug 2017, Jim Bride <jim.br...@linux.intel.com> wrote: > > 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 and v3: Rebase > > v4: * Fix up some leaky mode stuff (Chris) > > * Rebase > > v5: * Fix a NULL pointer derefrence (David Weinehall) > > v6: * Whitespace / spelling / checkpatch clean-up; no functional > > change. (David) > > * Rebase > > > > 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> > > Reviewed-by: David Weinehall <david.weineh...@linux.intel.com> > > Signed-off-by: Jim Bride <jim.br...@linux.intel.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, 45 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 76c8a0b..576b5af 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1606,6 +1606,23 @@ 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; > > What is bres?
boolean result :) > > + > > + 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; > > You know you could just do > > return m1 && m2 && m1->hdisplay == m2->hdisplay && ... Yeah, but I think the above is a bit more readable, and also makes it easier to add debug instrumentation at some point if needed. Jim > But I'm nitpicking. > > BR, > Jani. > > > > +} > > + > > bool > > intel_dp_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config, > > @@ -1652,8 +1669,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; > > @@ -5780,6 +5805,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; > > @@ -5835,13 +5861,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 if (!alt_fixed_mode) { > > + alt_fixed_mode = drm_mode_duplicate(dev, scan); > > } > > } > > > > @@ -5878,7 +5905,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 f91de9c..b3fcbb9 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 */ > > @@ -1679,6 +1680,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 b0b3adf..f0c11ae 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -1849,7 +1849,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 baf369d..c0a0272 100644 > > --- a/drivers/gpu/drm/i915/intel_dvo.c > > +++ b/drivers/gpu/drm/i915/intel_dvo.c > > @@ -552,7 +552,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 61d5579..8e21577 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -1138,7 +1138,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 88018fc..a17b1de 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -1920,11 +1920,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; > > @@ -1938,6 +1940,10 @@ 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); > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx