[v2,4/9] drm/i915: Add reverse mapping between port and intel_encoder

Submitted by Takashi Iwai on Dec. 1, 2015, 4:09 p.m.

Details

Message ID 1448986198-3488-5-git-send-email-tiwai@suse.de
State New
Headers show
Series "Add get_eld audio component for i915/HD-audio" ( rev: 3 2 ) in Intel GFX

Commit Message

Takashi Iwai Dec. 1, 2015, 4:09 p.m.
This patch adds a reverse mapping from a digital port number to
intel_encoder object containing the corresponding intel_digital_port.
It simplifies the query of the encoder a lot.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/i915/i915_drv.h    |  2 ++
 drivers/gpu/drm/i915/intel_audio.c | 22 ++--------------------
 drivers/gpu/drm/i915/intel_ddi.c   |  1 +
 drivers/gpu/drm/i915/intel_dp.c    |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c  |  2 ++
 5 files changed, 8 insertions(+), 20 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 95bb27de774f..3483d8125eac 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1955,6 +1955,8 @@  struct drm_i915_private {
 	/* perform PHY state sanity checks? */
 	bool chv_phy_assert[2];
 
+	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 19958bdb9bd0..67ee99f00ddd 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -630,28 +630,10 @@  static int i915_audio_component_get_cdclk_freq(struct device *dev)
 	return ret;
 }
 
-static struct intel_encoder *audio_port_to_encoder(struct drm_device *drm_dev,
-						   int port)
-{
-	struct intel_encoder *intel_encoder;
-	struct intel_digital_port *intel_dig_port;
-
-	for_each_intel_encoder(drm_dev, intel_encoder) {
-		if (intel_encoder->type != INTEL_OUTPUT_HDMI &&
-		    intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)
-			continue;
-		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
-		if (port == intel_dig_port->port)
-			return intel_encoder;
-	}
-	return NULL;
-}
-
 static int i915_audio_component_sync_audio_rate(struct device *dev,
 						int port, int rate)
 {
 	struct drm_i915_private *dev_priv = dev_to_i915(dev);
-	struct drm_device *drm_dev = dev_priv->dev;
 	struct intel_encoder *intel_encoder;
 	struct intel_crtc *crtc;
 	struct drm_display_mode *mode;
@@ -668,7 +650,7 @@  static int i915_audio_component_sync_audio_rate(struct device *dev,
 
 	mutex_lock(&dev_priv->av_mutex);
 	/* 1. get the pipe */
-	intel_encoder = audio_port_to_encoder(drm_dev, port);
+	intel_encoder = dev_priv->dig_port_map[port];
 	if (!intel_encoder || intel_encoder->type != INTEL_OUTPUT_HDMI) {
 		DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
 		mutex_unlock(&dev_priv->av_mutex);
@@ -725,7 +707,7 @@  static int i915_audio_component_get_eld(struct device *dev, int port,
 	int ret = -EINVAL;
 
 	drm_modeset_lock_all(drm_dev);
-	intel_encoder = audio_port_to_encoder(drm_dev, port);
+	intel_encoder = dev_priv->dig_port_map[port];
 	if (intel_encoder) {
 		ret = 0;
 		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a6752a61d99f..6770110a4075 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3285,6 +3285,7 @@  void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_encoder->get_config = intel_ddi_get_config;
 
 	intel_dig_port->port = port;
+	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 09bdd94ca3ba..1c02c6466f30 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6201,6 +6201,7 @@  intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	}
 
 	intel_dig_port->port = port;
+	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->dp.output_reg = output_reg;
 
 	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 9eafa191cee2..1d9f522a6679 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2133,6 +2133,7 @@  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 
 void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
@@ -2201,6 +2202,7 @@  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
 		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
 
 	intel_dig_port->port = port;
+	dev_priv->dig_port_map[port] = intel_encoder;
 	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
 	intel_dig_port->dp.output_reg = 0;
 

Comments

On Tue, 01 Dec 2015 17:09:53 +0100,
Takashi Iwai wrote:
> 
> This patch adds a reverse mapping from a digital port number to
> intel_encoder object containing the corresponding intel_digital_port.
> It simplifies the query of the encoder a lot.

While this is good for a code reduction, I guess it's better to leave
away for now, as there will be more changes there for MST support.
It may put yet another loop, and the mapping implemented here might
not be the best way.  So I'm going to drop this from the next
patchset.

If anyone still thinks this is worth to include, let me know.  I'll
re-add this.


thanks,

Takashi

> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    |  2 ++
>  drivers/gpu/drm/i915/intel_audio.c | 22 ++--------------------
>  drivers/gpu/drm/i915/intel_ddi.c   |  1 +
>  drivers/gpu/drm/i915/intel_dp.c    |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c  |  2 ++
>  5 files changed, 8 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 95bb27de774f..3483d8125eac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1955,6 +1955,8 @@ struct drm_i915_private {
>  	/* perform PHY state sanity checks? */
>  	bool chv_phy_assert[2];
>  
> +	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
> +
>  	/*
>  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>  	 * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 19958bdb9bd0..67ee99f00ddd 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -630,28 +630,10 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
>  	return ret;
>  }
>  
> -static struct intel_encoder *audio_port_to_encoder(struct drm_device *drm_dev,
> -						   int port)
> -{
> -	struct intel_encoder *intel_encoder;
> -	struct intel_digital_port *intel_dig_port;
> -
> -	for_each_intel_encoder(drm_dev, intel_encoder) {
> -		if (intel_encoder->type != INTEL_OUTPUT_HDMI &&
> -		    intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)
> -			continue;
> -		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> -		if (port == intel_dig_port->port)
> -			return intel_encoder;
> -	}
> -	return NULL;
> -}
> -
>  static int i915_audio_component_sync_audio_rate(struct device *dev,
>  						int port, int rate)
>  {
>  	struct drm_i915_private *dev_priv = dev_to_i915(dev);
> -	struct drm_device *drm_dev = dev_priv->dev;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_crtc *crtc;
>  	struct drm_display_mode *mode;
> @@ -668,7 +650,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  	/* 1. get the pipe */
> -	intel_encoder = audio_port_to_encoder(drm_dev, port);
> +	intel_encoder = dev_priv->dig_port_map[port];
>  	if (!intel_encoder || intel_encoder->type != INTEL_OUTPUT_HDMI) {
>  		DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
>  		mutex_unlock(&dev_priv->av_mutex);
> @@ -725,7 +707,7 @@ static int i915_audio_component_get_eld(struct device *dev, int port,
>  	int ret = -EINVAL;
>  
>  	drm_modeset_lock_all(drm_dev);
> -	intel_encoder = audio_port_to_encoder(drm_dev, port);
> +	intel_encoder = dev_priv->dig_port_map[port];
>  	if (intel_encoder) {
>  		ret = 0;
>  		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index a6752a61d99f..6770110a4075 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3285,6 +3285,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_encoder->get_config = intel_ddi_get_config;
>  
>  	intel_dig_port->port = port;
> +	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>  					  (DDI_BUF_PORT_REVERSAL |
>  					   DDI_A_4_LANES);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 09bdd94ca3ba..1c02c6466f30 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6201,6 +6201,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	}
>  
>  	intel_dig_port->port = port;
> +	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->dp.output_reg = output_reg;
>  
>  	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 9eafa191cee2..1d9f522a6679 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2133,6 +2133,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  
>  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_connector *intel_connector;
> @@ -2201,6 +2202,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
>  		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
>  
>  	intel_dig_port->port = port;
> +	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
>  	intel_dig_port->dp.output_reg = 0;
>  
> -- 
> 2.6.3
>
On Fri, Dec 04, 2015 at 03:40:03PM +0100, Takashi Iwai wrote:
> On Tue, 01 Dec 2015 17:09:53 +0100,
> Takashi Iwai wrote:
> > 
> > This patch adds a reverse mapping from a digital port number to
> > intel_encoder object containing the corresponding intel_digital_port.
> > It simplifies the query of the encoder a lot.
> 
> While this is good for a code reduction, I guess it's better to leave
> away for now, as there will be more changes there for MST support.
> It may put yet another loop, and the mapping implemented here might
> not be the best way.  So I'm going to drop this from the next
> patchset.
> 
> If anyone still thinks this is worth to include, let me know.  I'll
> re-add this.

Yeah I think this looks good. Fixing up conflicts with MST shouldn't be
that much trouble really.
-Daniel

> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h    |  2 ++
> >  drivers/gpu/drm/i915/intel_audio.c | 22 ++--------------------
> >  drivers/gpu/drm/i915/intel_ddi.c   |  1 +
> >  drivers/gpu/drm/i915/intel_dp.c    |  1 +
> >  drivers/gpu/drm/i915/intel_hdmi.c  |  2 ++
> >  5 files changed, 8 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 95bb27de774f..3483d8125eac 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1955,6 +1955,8 @@ struct drm_i915_private {
> >  	/* perform PHY state sanity checks? */
> >  	bool chv_phy_assert[2];
> >  
> > +	struct intel_encoder *dig_port_map[I915_MAX_PORTS];
> > +
> >  	/*
> >  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >  	 * will be rejected. Instead look for a better place.
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 19958bdb9bd0..67ee99f00ddd 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -630,28 +630,10 @@ static int i915_audio_component_get_cdclk_freq(struct device *dev)
> >  	return ret;
> >  }
> >  
> > -static struct intel_encoder *audio_port_to_encoder(struct drm_device *drm_dev,
> > -						   int port)
> > -{
> > -	struct intel_encoder *intel_encoder;
> > -	struct intel_digital_port *intel_dig_port;
> > -
> > -	for_each_intel_encoder(drm_dev, intel_encoder) {
> > -		if (intel_encoder->type != INTEL_OUTPUT_HDMI &&
> > -		    intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)
> > -			continue;
> > -		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> > -		if (port == intel_dig_port->port)
> > -			return intel_encoder;
> > -	}
> > -	return NULL;
> > -}
> > -
> >  static int i915_audio_component_sync_audio_rate(struct device *dev,
> >  						int port, int rate)
> >  {
> >  	struct drm_i915_private *dev_priv = dev_to_i915(dev);
> > -	struct drm_device *drm_dev = dev_priv->dev;
> >  	struct intel_encoder *intel_encoder;
> >  	struct intel_crtc *crtc;
> >  	struct drm_display_mode *mode;
> > @@ -668,7 +650,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
> >  
> >  	mutex_lock(&dev_priv->av_mutex);
> >  	/* 1. get the pipe */
> > -	intel_encoder = audio_port_to_encoder(drm_dev, port);
> > +	intel_encoder = dev_priv->dig_port_map[port];
> >  	if (!intel_encoder || intel_encoder->type != INTEL_OUTPUT_HDMI) {
> >  		DRM_DEBUG_KMS("no pipe for the port %c\n", port_name(port));
> >  		mutex_unlock(&dev_priv->av_mutex);
> > @@ -725,7 +707,7 @@ static int i915_audio_component_get_eld(struct device *dev, int port,
> >  	int ret = -EINVAL;
> >  
> >  	drm_modeset_lock_all(drm_dev);
> > -	intel_encoder = audio_port_to_encoder(drm_dev, port);
> > +	intel_encoder = dev_priv->dig_port_map[port];
> >  	if (intel_encoder) {
> >  		ret = 0;
> >  		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index a6752a61d99f..6770110a4075 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3285,6 +3285,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  	intel_encoder->get_config = intel_ddi_get_config;
> >  
> >  	intel_dig_port->port = port;
> > +	dev_priv->dig_port_map[port] = intel_encoder;
> >  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
> >  					  (DDI_BUF_PORT_REVERSAL |
> >  					   DDI_A_4_LANES);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 09bdd94ca3ba..1c02c6466f30 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6201,6 +6201,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> >  	}
> >  
> >  	intel_dig_port->port = port;
> > +	dev_priv->dig_port_map[port] = intel_encoder;
> >  	intel_dig_port->dp.output_reg = output_reg;
> >  
> >  	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 9eafa191cee2..1d9f522a6679 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -2133,6 +2133,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  
> >  void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
> >  {
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct intel_digital_port *intel_dig_port;
> >  	struct intel_encoder *intel_encoder;
> >  	struct intel_connector *intel_connector;
> > @@ -2201,6 +2202,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
> >  		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
> >  
> >  	intel_dig_port->port = port;
> > +	dev_priv->dig_port_map[port] = intel_encoder;
> >  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
> >  	intel_dig_port->dp.output_reg = 0;
> >  
> > -- 
> > 2.6.3
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 04 Dec 2015 16:02:29 +0100,
Daniel Vetter wrote:
> 
> On Fri, Dec 04, 2015 at 03:40:03PM +0100, Takashi Iwai wrote:
> > On Tue, 01 Dec 2015 17:09:53 +0100,
> > Takashi Iwai wrote:
> > > 
> > > This patch adds a reverse mapping from a digital port number to
> > > intel_encoder object containing the corresponding intel_digital_port.
> > > It simplifies the query of the encoder a lot.
> > 
> > While this is good for a code reduction, I guess it's better to leave
> > away for now, as there will be more changes there for MST support.
> > It may put yet another loop, and the mapping implemented here might
> > not be the best way.  So I'm going to drop this from the next
> > patchset.
> > 
> > If anyone still thinks this is worth to include, let me know.  I'll
> > re-add this.
> 
> Yeah I think this looks good. Fixing up conflicts with MST shouldn't be
> that much trouble really.

Alright, resurrected from ash.


Takashi