[3/4] drm/i915: ELiminate intel_pipe_to_cpu_transcoder() from assert_fdi_tx()
diff mbox series

Message ID 20191112163812.22075-3-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • [1/4] drm/i915/fbc: Nuke bogus single pipe fbc1 restriction
Related show

Commit Message

Ville Syrjälä Nov. 12, 2019, 4:38 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's start to eliminate intel_pipe_to_cpu_transcoder() so that
we can get rid of one more crtc->config usage (which we will want
to nuke as well).

In the case of assert_fdi_tx() we know that we're never
dealing with the EDP transcoder so we can simply replace
this with a cast.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

José Roberto de Souza Dec. 10, 2019, 6:19 p.m. UTC | #1
On Tue, 2019-11-12 at 18:38 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Let's start to eliminate intel_pipe_to_cpu_transcoder() so that
> we can get rid of one more crtc->config usage (which we will want
> to nuke as well).
> 
> In the case of assert_fdi_tx() we know that we're never
> dealing with the EDP transcoder so we can simply replace
> this with a cast.

There still one EDP transcoder case when running in a HSW:

haswell_crtc_enable()
	lpt_pch_enable()
		lpt_enable_pch_transcoder()
			assert_fdi_tx_enabled()

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e7e5497e6f2e..cabd88337822 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1141,11 +1141,15 @@ static void assert_fdi_tx(struct
> drm_i915_private *dev_priv,
>  			  enum pipe pipe, bool state)
>  {
>  	bool cur_state;
> -	enum transcoder cpu_transcoder =
> intel_pipe_to_cpu_transcoder(dev_priv,
> -								      p
> ipe);
>  
>  	if (HAS_DDI(dev_priv)) {
> -		/* DDI does not have a specific FDI_TX register */
> +		/*
> +		 * DDI does not have a specific FDI_TX register.
> +		 *
> +		 * FDI is never hooked fed from EDP transcoder
> +		 * so pipe->transcoder cast is fine here.
> +		 */
> +		enum transcoder cpu_transcoder = (enum transcoder)pipe;
>  		u32 val =
> I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  		cur_state = !!(val & TRANS_DDI_FUNC_ENABLE);
>  	} else {
Ville Syrjälä Dec. 10, 2019, 7:55 p.m. UTC | #2
On Tue, Dec 10, 2019 at 06:19:24PM +0000, Souza, Jose wrote:
> On Tue, 2019-11-12 at 18:38 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Let's start to eliminate intel_pipe_to_cpu_transcoder() so that
> > we can get rid of one more crtc->config usage (which we will want
> > to nuke as well).
> > 
> > In the case of assert_fdi_tx() we know that we're never
> > dealing with the EDP transcoder so we can simply replace
> > this with a cast.
> 
> There still one EDP transcoder case when running in a HSW:
> 
> haswell_crtc_enable()
> 	lpt_pch_enable()
> 		lpt_enable_pch_transcoder()
> 			assert_fdi_tx_enabled()

The transcoder will never EDP. But this casting back and forth 
is a bit ugly. Maybe I should just kill these FDI TX asserts
from the hsw/bdw code...

> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index e7e5497e6f2e..cabd88337822 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1141,11 +1141,15 @@ static void assert_fdi_tx(struct
> > drm_i915_private *dev_priv,
> >  			  enum pipe pipe, bool state)
> >  {
> >  	bool cur_state;
> > -	enum transcoder cpu_transcoder =
> > intel_pipe_to_cpu_transcoder(dev_priv,
> > -								      p
> > ipe);
> >  
> >  	if (HAS_DDI(dev_priv)) {
> > -		/* DDI does not have a specific FDI_TX register */
> > +		/*
> > +		 * DDI does not have a specific FDI_TX register.
> > +		 *
> > +		 * FDI is never hooked fed from EDP transcoder
> > +		 * so pipe->transcoder cast is fine here.
> > +		 */
> > +		enum transcoder cpu_transcoder = (enum transcoder)pipe;
> >  		u32 val =
> > I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >  		cur_state = !!(val & TRANS_DDI_FUNC_ENABLE);
> >  	} else {
José Roberto de Souza Dec. 10, 2019, 8:10 p.m. UTC | #3
On Tue, 2019-12-10 at 21:55 +0200, Ville Syrjälä wrote:
> On Tue, Dec 10, 2019 at 06:19:24PM +0000, Souza, Jose wrote:
> > On Tue, 2019-11-12 at 18:38 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Let's start to eliminate intel_pipe_to_cpu_transcoder() so that
> > > we can get rid of one more crtc->config usage (which we will want
> > > to nuke as well).
> > > 
> > > In the case of assert_fdi_tx() we know that we're never
> > > dealing with the EDP transcoder so we can simply replace
> > > this with a cast.
> > 
> > There still one EDP transcoder case when running in a HSW:
> > 
> > haswell_crtc_enable()
> > 	lpt_pch_enable()
> > 		lpt_enable_pch_transcoder()
> > 			assert_fdi_tx_enabled()
> 
> The transcoder will never EDP. But this casting back and forth 
> is a bit ugly. Maybe I should just kill these FDI TX asserts
> from the hsw/bdw code...

Oh misread crt with crtc, so has_pch_encoder will not be set for edp.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

> 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index e7e5497e6f2e..cabd88337822 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -1141,11 +1141,15 @@ static void assert_fdi_tx(struct
> > > drm_i915_private *dev_priv,
> > >  			  enum pipe pipe, bool state)
> > >  {
> > >  	bool cur_state;
> > > -	enum transcoder cpu_transcoder =
> > > intel_pipe_to_cpu_transcoder(dev_priv,
> > > -								      p
> > > ipe);
> > >  
> > >  	if (HAS_DDI(dev_priv)) {
> > > -		/* DDI does not have a specific FDI_TX register */
> > > +		/*
> > > +		 * DDI does not have a specific FDI_TX register.
> > > +		 *
> > > +		 * FDI is never hooked fed from EDP transcoder
> > > +		 * so pipe->transcoder cast is fine here.
> > > +		 */
> > > +		enum transcoder cpu_transcoder = (enum transcoder)pipe;
> > >  		u32 val =
> > > I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > >  		cur_state = !!(val & TRANS_DDI_FUNC_ENABLE);
> > >  	} else {

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e7e5497e6f2e..cabd88337822 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1141,11 +1141,15 @@  static void assert_fdi_tx(struct drm_i915_private *dev_priv,
 			  enum pipe pipe, bool state)
 {
 	bool cur_state;
-	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
-								      pipe);
 
 	if (HAS_DDI(dev_priv)) {
-		/* DDI does not have a specific FDI_TX register */
+		/*
+		 * DDI does not have a specific FDI_TX register.
+		 *
+		 * FDI is never hooked fed from EDP transcoder
+		 * so pipe->transcoder cast is fine here.
+		 */
+		enum transcoder cpu_transcoder = (enum transcoder)pipe;
 		u32 val = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 		cur_state = !!(val & TRANS_DDI_FUNC_ENABLE);
 	} else {