Re: [PATCH 5/5] drm/i915: add DP aux ctl regs to struct intel_dp

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Dec 06, 2012 at 04:53:22PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So we can just reuse the value whenever we need, reducing code
> duplication.
> 
> V2: Rebase since patch 3/5 was discarded.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Since Haswel && !PORT_A is the only special case where the dp aux stuff
isn't at a fixed offset, I think we can condense this down a lot and rip
out all the FAKE stuff. Also, since dp aux data&control are everywhere at
fixed offsets relative to each another, I don't think we need them both.

Merged patches 1,2&4 from this series, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 12 +++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 56 +++++++---------------------------------
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 24 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f2a5ea6..4307cb2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2541,6 +2541,9 @@
>  #define DPD_AUX_CH_DATA4		0x64320
>  #define DPD_AUX_CH_DATA5		0x64324
>  
> +#define DP_AUX_CH_CTL(port)    _PORT(port, DPA_AUX_CH_CTL, DPB_AUX_CH_CTL)
> +#define DP_AUX_CH_DATA1(port)  _PORT(port, DPA_AUX_CH_DATA1, DPB_AUX_CH_DATA1)
> +
>  #define   DP_AUX_CH_CTL_SEND_BUSY	    (1 << 31)
>  #define   DP_AUX_CH_CTL_DONE		    (1 << 30)
>  #define   DP_AUX_CH_CTL_INTERRUPT	    (1 << 29)
> @@ -4066,6 +4069,15 @@
>  #define PCH_DPD_AUX_CH_DATA4	0xe4320
>  #define PCH_DPD_AUX_CH_DATA5	0xe4324
>  
> +/* Definitions to make the macro easier to understand: */
> +#define __FAKE_PCH_DPA_AUX_CH_CTL	0xe4010
> +#define __FAKE_PCH_DPA_AUX_CH_DATA1	0xe4014
> +
> +#define PCH_DP_AUX_CH_CTL(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_CTL, \
> +				      PCH_DPB_AUX_CH_CTL)
> +#define PCH_DP_AUX_CH_DATA1(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_DATA1, \
> +					PCH_DPB_AUX_CH_DATA1)
> +
>  /* CPT */
>  #define  PORT_TRANS_A_SEL_CPT	0
>  #define  PORT_TRANS_B_SEL_CPT	(1<<29)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 75073cc..2db90f8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -328,29 +328,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t ch_ctl = intel_dp->output_reg + 0x10;
> +	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
>  	uint32_t status;
>  	bool done;
>  
> -	if (IS_HASWELL(dev)) {
> -		switch (intel_dig_port->port) {
> -		case PORT_A:
> -			ch_ctl = DPA_AUX_CH_CTL;
> -			break;
> -		case PORT_B:
> -			ch_ctl = PCH_DPB_AUX_CH_CTL;
> -			break;
> -		case PORT_C:
> -			ch_ctl = PCH_DPC_AUX_CH_CTL;
> -			break;
> -		case PORT_D:
> -			ch_ctl = PCH_DPD_AUX_CH_CTL;
> -			break;
> -		default:
> -			BUG();
> -		}
> -	}
> -
>  #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
>  	if (has_aux_irq)
>  		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
> @@ -369,12 +350,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		uint8_t *send, int send_bytes,
>  		uint8_t *recv, int recv_size)
>  {
> -	uint32_t output_reg = intel_dp->output_reg;
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t ch_ctl = output_reg + 0x10;
> -	uint32_t ch_data = ch_ctl + 4;
> +	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
> +	uint32_t ch_data = intel_dp->aux_ch_data_reg;
>  	int i, ret, recv_bytes;
>  	uint32_t status;
>  	uint32_t aux_clock_divider;
> @@ -387,29 +367,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	 */
>  	pm_qos_update_request(&dev_priv->pm_qos, 0);
>  
> -	if (IS_HASWELL(dev)) {
> -		switch (intel_dig_port->port) {
> -		case PORT_A:
> -			ch_ctl = DPA_AUX_CH_CTL;
> -			ch_data = DPA_AUX_CH_DATA1;
> -			break;
> -		case PORT_B:
> -			ch_ctl = PCH_DPB_AUX_CH_CTL;
> -			ch_data = PCH_DPB_AUX_CH_DATA1;
> -			break;
> -		case PORT_C:
> -			ch_ctl = PCH_DPC_AUX_CH_CTL;
> -			ch_data = PCH_DPC_AUX_CH_DATA1;
> -			break;
> -		case PORT_D:
> -			ch_ctl = PCH_DPD_AUX_CH_CTL;
> -			ch_data = PCH_DPD_AUX_CH_DATA1;
> -			break;
> -		default:
> -			BUG();
> -		}
> -	}
> -
>  	intel_dp_check_edp(intel_dp);
>  	/* The clock divider is based off the hrawclk,
>  	 * and would like to run at 2MHz. So, take the
> @@ -2786,6 +2743,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	else
>  		intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
> +	if (port == PORT_A || INTEL_INFO(dev)->gen < 5) {
> +		intel_dp->aux_ch_ctl_reg = DP_AUX_CH_CTL(port);
> +		intel_dp->aux_ch_data_reg = DP_AUX_CH_DATA1(port);
> +	} else {
> +		intel_dp->aux_ch_ctl_reg = PCH_DP_AUX_CH_CTL(port);
> +		intel_dp->aux_ch_data_reg = PCH_DP_AUX_CH_DATA1(port);
> +	}
>  
>  	/* Set up the DDC bus. */
>  	switch (port) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7ca7772..aebb023 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -352,6 +352,8 @@ struct intel_hdmi {
>  
>  struct intel_dp {
>  	uint32_t output_reg;
> +	uint32_t aux_ch_ctl_reg;
> +	uint32_t aux_ch_data_reg;
>  	uint32_t DP;
>  	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
>  	bool has_audio;
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch