[Intel-gfx] [PATCH v6] drm/i915/edp: Allow alternate fixed mode for eDP if available.

Jim Bride jim.bride at linux.intel.com
Thu Aug 10 16:37:37 UTC 2017


On Thu, Aug 10, 2017 at 04:13:44PM +0300, Jani Nikula wrote:
> On Wed, 09 Aug 2017, Jim Bride <jim.bride at 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.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>
> > Reviewed-by: David Weinehall <david.weinehall at linux.intel.com>
> > Signed-off-by: Jim Bride <jim.bride at 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


More information about the Intel-gfx mailing list