Re: [PATCH] drm/i915: Add additional pipe parameter for vlv_dpio_read and vlv_dpio_write.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Sep 05, 2013 at 01:33:37PM +0800, Chon Ming Lee wrote:
> The additional pipe parameter will use to select which phy to target
> for.
> 
> Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |   18 ++++++------
>  drivers/gpu/drm/i915/i915_drv.h       |    4 +-
>  drivers/gpu/drm/i915/intel_display.c  |   51 +++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_dp.c       |   38 ++++++++++++++----------
>  drivers/gpu/drm/i915/intel_hdmi.c     |   48 +++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_sideband.c |   20 +++++++++---
>  6 files changed, 101 insertions(+), 78 deletions(-)
> 
<snip>
> diff --git a/drivers/gpu/drm/i915/intel_sideband.c b/drivers/gpu/drm/i915/intel_sideband.c
> index 0a41670..12bbc28 100644
> --- a/drivers/gpu/drm/i915/intel_sideband.c
> +++ b/drivers/gpu/drm/i915/intel_sideband.c
> @@ -157,19 +157,29 @@ void vlv_gps_core_write(struct drm_i915_private *dev_priv, u32 reg, u32 val)
>  			PUNIT_OPCODE_REG_WRITE, reg, &val);
>  }
>  
> -u32 vlv_dpio_read(struct drm_i915_private *dev_priv, int reg)
> +u32 vlv_get_phy_port(enum pipe pipe)

This thing isn't used elsewhere, so it should be static. I think sparse
would complain about such things, so it's a good idea to run it over
any patches you're about to send out.

> +{
> +	u32 port = IOSF_PORT_DPIO;
> +
> +	if (!((pipe == PIPE_A) || (pipe == PIPE_B))) {
> +		DRM_ERROR("Invalid pipe detected\n");
> +	}

Could just be something like WARN_ON(pipe != PIPE_A && pipe != PIPE_B)
Though I'm not entirely sure the check is actually useful.

But anyway the rest of the patch looks OK to me, and I'm assuming gcc
reviewed it for you in detail already, so:

Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	return port;
> +}
> +
> +u32 vlv_dpio_read(struct drm_i915_private *dev_priv, enum pipe pipe, int reg)
>  {
>  	u32 val = 0;
>  
> -	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
> +	vlv_sideband_rw(dev_priv, DPIO_DEVFN, vlv_get_phy_port(pipe),
>  			DPIO_OPCODE_REG_READ, reg, &val);
> -
>  	return val;
>  }
>  
> -void vlv_dpio_write(struct drm_i915_private *dev_priv, int reg, u32 val)
> +void vlv_dpio_write(struct drm_i915_private *dev_priv, enum pipe pipe, int reg, u32 val)
>  {
> -	vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_DPIO,
> +	vlv_sideband_rw(dev_priv, DPIO_DEVFN, vlv_get_phy_port(pipe),
>  			DPIO_OPCODE_REG_WRITE, reg, &val);
>  }
>  
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC