diff mbox

[09/10] drm/i915: Use intel_ddi_get_config() for MST

Message ID 20171019133721.11794-10-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 19, 2017, 1:37 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Eliminate the partially duplicated DDI readout code from MST, and
instead just call intel_ddi_get_config(). As a nice bonus we get
more cross checking as intel_ddi_get_config() will populate
output_types based on the actual mode of the DDI port.

Additonally intel_ddi_get_config() must be changed to get the crtc
from the passed in crtc state rather than from the encoder->crtc link.
encoder->crtc really shouldn't be used anyway.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    |  2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c | 44 +------------------------------------
 2 files changed, 2 insertions(+), 44 deletions(-)

Comments

Maarten Lankhorst Oct. 27, 2017, 11:43 a.m. UTC | #1
Op 19-10-17 om 15:37 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Eliminate the partially duplicated DDI readout code from MST, and
> instead just call intel_ddi_get_config(). As a nice bonus we get
> more cross checking as intel_ddi_get_config() will populate
> output_types based on the actual mode of the DDI port.
>
> Additonally intel_ddi_get_config() must be changed to get the crtc
> from the passed in crtc state rather than from the encoder->crtc link.
> encoder->crtc really shouldn't be used anyway.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I looked and noticed 2 small differences, one is the GEN9_LP part, the other is
min_voltage_level. Is this also a bugfix because of this?

Cheers,
Maarten
Ville Syrjälä Oct. 27, 2017, 11:55 a.m. UTC | #2
On Fri, Oct 27, 2017 at 01:43:03PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Eliminate the partially duplicated DDI readout code from MST, and
> > instead just call intel_ddi_get_config(). As a nice bonus we get
> > more cross checking as intel_ddi_get_config() will populate
> > output_types based on the actual mode of the DDI port.
> >
> > Additonally intel_ddi_get_config() must be changed to get the crtc
> > from the passed in crtc state rather than from the encoder->crtc link.
> > encoder->crtc really shouldn't be used anyway.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> I looked and noticed 2 small differences, one is the GEN9_LP part, the other is
> min_voltage_level. Is this also a bugfix because of this?

The min_voltage thing should be fine. We should have it in both DDI and
MST code (unless I made a mistake when adding it, but doesn't look like
I did).

The bxt lat_optim thing is a real issue. Looks like I need to add that to
the MST compute_config. Though as a whole it feels a bit overkill to
track that particular piece of state specifically. We don't do that on
CHV either, which has the exact same PHY.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0123c4753cd4..e96b458d6592 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2507,7 +2507,7 @@  void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	struct intel_digital_port *intel_dig_port;
 	u32 temp, flags = 0;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index cdffe7e93a71..e9584294dbbe 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -265,50 +265,8 @@  static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
-	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	u32 temp, flags = 0;
 
-	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
-
-	pipe_config->has_audio =
-		intel_ddi_is_audio_enabled(dev_priv, crtc);
-
-	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
-	if (temp & TRANS_DDI_PHSYNC)
-		flags |= DRM_MODE_FLAG_PHSYNC;
-	else
-		flags |= DRM_MODE_FLAG_NHSYNC;
-	if (temp & TRANS_DDI_PVSYNC)
-		flags |= DRM_MODE_FLAG_PVSYNC;
-	else
-		flags |= DRM_MODE_FLAG_NVSYNC;
-
-	switch (temp & TRANS_DDI_BPC_MASK) {
-	case TRANS_DDI_BPC_6:
-		pipe_config->pipe_bpp = 18;
-		break;
-	case TRANS_DDI_BPC_8:
-		pipe_config->pipe_bpp = 24;
-		break;
-	case TRANS_DDI_BPC_10:
-		pipe_config->pipe_bpp = 30;
-		break;
-	case TRANS_DDI_BPC_12:
-		pipe_config->pipe_bpp = 36;
-		break;
-	default:
-		break;
-	}
-	pipe_config->base.adjusted_mode.flags |= flags;
-
-	pipe_config->lane_count =
-		((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
-
-	intel_dp_get_m_n(crtc, pipe_config);
-
-	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
+	intel_ddi_get_config(&intel_dig_port->base, pipe_config);
 }
 
 static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)