diff mbox series

[2/2] drm/i915/tgl: Fix the read of the DDI that transcoder is attached to

Message ID 20190801232812.28375-2-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915: Get transcoder power domain before reading its register | expand

Commit Message

Souza, Jose Aug. 1, 2019, 11:28 p.m. UTC 
On TGL this register do not map directly to port, it was already
handled when setting it(TGL_TRANS_DDI_SELECT_PORT()) but not when
reading it.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Souza, Jose Aug. 2, 2019, 1:07 a.m. UTC | #1 
On Thu, 2019-08-01 at 17:50 -0700, Lucas De Marchi wrote:
> On Thu, Aug 01, 2019 at 04:28:12PM -0700, Jose Souza wrote:
> > On TGL this register do not map directly to port, it was already
> > handled when setting it(TGL_TRANS_DDI_SELECT_PORT()) but not when
> > reading it.
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 9e4ee29fd0fc..b9526aa402f9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -10353,11 +10353,17 @@ static void
> > haswell_get_ddi_port_state(struct intel_crtc *crtc,
> > 
> > 	tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config-
> > >cpu_transcoder));
> > 
> > -	if (INTEL_GEN(dev_priv) >= 12)
> > +	if (INTEL_GEN(dev_priv) >= 12) {
> > 		port = (tmp & TGL_TRANS_DDI_PORT_MASK) >>
> > 			TGL_TRANS_DDI_PORT_SHIFT;
> > -	else
> > +		/*
> > +		 * Register values: none = 0, DDIA = 1... while PORT_A
> > = 0...
> > +		 * so subtract one
> > +		 */
> > +		port--;
> 
> port = TGL_PORT_TRANS_DDI_SELECT(tmp)
> 
> and put the macro right below the TGL_TRANS_DDI_SELECT_PORT() so the
> intent is explicit and we don't forget again. Then you can remove the
> comment.

I liked the idea of add a macro but not sure about this name, going to
think in a better one.

> 
> any chance of tmp being none and the -1 underflow?

I guess the intention of have 0 = none is DP MST but we are programing
the DDI_SELECT even on slaves, if we stop to do that we would need to
do changes in this function.

> 
> Lucas De Marchi
> 
> 
> > +	} else {
> > 		port = (tmp & TRANS_DDI_PORT_MASK) >>
> > TRANS_DDI_PORT_SHIFT;
> > +	}
> > 
> > 	if (INTEL_GEN(dev_priv) >= 11)
> > 		icelake_get_ddi_pll(dev_priv, port, pipe_config);
> > -- 
> > 2.22.0
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 9e4ee29fd0fc..b9526aa402f9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10353,11 +10353,17 @@  static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 
 	tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config->cpu_transcoder));
 
-	if (INTEL_GEN(dev_priv) >= 12)
+	if (INTEL_GEN(dev_priv) >= 12) {
 		port = (tmp & TGL_TRANS_DDI_PORT_MASK) >>
 			TGL_TRANS_DDI_PORT_SHIFT;
-	else
+		/*
+		 * Register values: none = 0, DDIA = 1... while PORT_A = 0...
+		 * so subtract one
+		 */
+		port--;
+	} else {
 		port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
+	}
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		icelake_get_ddi_pll(dev_priv, port, pipe_config);