diff mbox

[v2] drm/i915: Remove mostly duplicated video DIP handling from PSR code

Message ID 20171013194051.19286-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 13, 2017, 7:40 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that the infoframe hooks are part of the intel_dig_port, we can use
the normal .write_infoframe() hook to update the VSC SDP. We do need to
deal with the size difference between the VSC DIP and the others though.

Another minor snag is that the compiler will complain to use if we keep
using enum hdmi_infoframe_type type and passing in the DP define instead,
so et's just change to unsigned int all over for the inforframe type.

v2: Rebase due to other PSR changes

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c | 26 ++++++++++++++++----------
 drivers/gpu/drm/i915/intel_psr.c  | 34 ++--------------------------------
 3 files changed, 19 insertions(+), 43 deletions(-)

Comments

Ville Syrjälä Oct. 17, 2017, 9:46 a.m. UTC | #1 
On Fri, Oct 13, 2017 at 10:40:51PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that the infoframe hooks are part of the intel_dig_port, we can use
> the normal .write_infoframe() hook to update the VSC SDP. We do need to
> deal with the size difference between the VSC DIP and the others though.
> 
> Another minor snag is that the compiler will complain to use if we keep
> using enum hdmi_infoframe_type type and passing in the DP define instead,
> so et's just change to unsigned int all over for the inforframe type.
> 
> v2: Rebase due to other PSR changes
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed to dinq. Thanks for the review.

> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c | 26 ++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_psr.c  | 34 ++--------------------------------
>  3 files changed, 19 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d61985f93d40..86eed3c7828b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1069,7 +1069,7 @@ struct intel_digital_port {
>  
>  	void (*write_infoframe)(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>  				const void *frame, ssize_t len);
>  	void (*set_infoframes)(struct drm_encoder *encoder,
>  			       bool enable,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e6f8f30ce7bd..5132dc814788 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -70,7 +70,7 @@ static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
>  	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
>  }
>  
> -static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
> +static u32 g4x_infoframe_index(unsigned int type)
>  {
>  	switch (type) {
>  	case HDMI_INFOFRAME_TYPE_AVI:
> @@ -85,7 +85,7 @@ static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
>  	}
>  }
>  
> -static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
> +static u32 g4x_infoframe_enable(unsigned int type)
>  {
>  	switch (type) {
>  	case HDMI_INFOFRAME_TYPE_AVI:
> @@ -100,9 +100,11 @@ static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
>  	}
>  }
>  
> -static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
> +static u32 hsw_infoframe_enable(unsigned int type)
>  {
>  	switch (type) {
> +	case DP_SDP_VSC:
> +		return VIDEO_DIP_ENABLE_VSC_HSW;
>  	case HDMI_INFOFRAME_TYPE_AVI:
>  		return VIDEO_DIP_ENABLE_AVI_HSW;
>  	case HDMI_INFOFRAME_TYPE_SPD:
> @@ -118,10 +120,12 @@ static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
>  static i915_reg_t
>  hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>  		 enum transcoder cpu_transcoder,
> -		 enum hdmi_infoframe_type type,
> +		 unsigned int type,
>  		 int i)
>  {
>  	switch (type) {
> +	case DP_SDP_VSC:
> +		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
>  	case HDMI_INFOFRAME_TYPE_AVI:
>  		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
>  	case HDMI_INFOFRAME_TYPE_SPD:
> @@ -136,7 +140,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>  
>  static void g4x_write_infoframe(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>  				const void *frame, ssize_t len)
>  {
>  	const uint32_t *data = frame;
> @@ -191,7 +195,7 @@ static bool g4x_infoframe_enabled(struct drm_encoder *encoder,
>  
>  static void ibx_write_infoframe(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>  				const void *frame, ssize_t len)
>  {
>  	const uint32_t *data = frame;
> @@ -251,7 +255,7 @@ static bool ibx_infoframe_enabled(struct drm_encoder *encoder,
>  
>  static void cpt_write_infoframe(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>  				const void *frame, ssize_t len)
>  {
>  	const uint32_t *data = frame;
> @@ -309,7 +313,7 @@ static bool cpt_infoframe_enabled(struct drm_encoder *encoder,
>  
>  static void vlv_write_infoframe(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>  				const void *frame, ssize_t len)
>  {
>  	const uint32_t *data = frame;
> @@ -368,7 +372,7 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder,
>  
>  static void hsw_write_infoframe(struct drm_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state,
> -				enum hdmi_infoframe_type type,
> +				unsigned int type,
>  				const void *frame, ssize_t len)
>  {
>  	const uint32_t *data = frame;
> @@ -377,6 +381,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
>  	i915_reg_t data_reg;
> +	int data_size = type == DP_SDP_VSC ?
> +		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
>  	int i;
>  	u32 val = I915_READ(ctl_reg);
>  
> @@ -392,7 +398,7 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>  		data++;
>  	}
>  	/* Write every possible data byte to force correct ECC calculation. */
> -	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
> +	for (; i < data_size; i += 4)
>  		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
>  					    type, i >> 2), 0);
>  	mmiowb();
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 93b177cc4cbf..6e3b430fccdc 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -75,37 +75,6 @@ static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>  	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
>  }
>  
> -static void intel_psr_write_vsc(struct intel_dp *intel_dp,
> -				const struct edp_vsc_psr *vsc_psr)
> -{
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> -	struct drm_device *dev = dig_port->base.base.dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> -	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
> -	uint32_t *data = (uint32_t *) vsc_psr;
> -	unsigned int i;
> -
> -	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
> -	   the video DIP being updated before program video DIP data buffer
> -	   registers for DIP being updated. */
> -	I915_WRITE(ctl_reg, 0);
> -	POSTING_READ(ctl_reg);
> -
> -	for (i = 0; i < sizeof(*vsc_psr); i += 4) {
> -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
> -						   i >> 2), *data);
> -		data++;
> -	}
> -	for (; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4)
> -		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
> -						   i >> 2), 0);
> -
> -	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
> -	POSTING_READ(ctl_reg);
> -}
> -
>  static void vlv_psr_setup_vsc(struct intel_dp *intel_dp,
>  			      const struct intel_crtc_state *crtc_state)
>  {
> @@ -152,7 +121,8 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
>  		psr_vsc.sdp_header.HB3 = 0x8;
>  	}
>  
> -	intel_psr_write_vsc(intel_dp, &psr_vsc);
> +	intel_dig_port->write_infoframe(&intel_dig_port->base.base, crtc_state,
> +					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
>  }
>  
>  static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
> -- 
> 2.13.6
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d61985f93d40..86eed3c7828b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1069,7 +1069,7 @@  struct intel_digital_port {
 
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len);
 	void (*set_infoframes)(struct drm_encoder *encoder,
 			       bool enable,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e6f8f30ce7bd..5132dc814788 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -70,7 +70,7 @@  static struct intel_hdmi *intel_attached_hdmi(struct drm_connector *connector)
 	return enc_to_intel_hdmi(&intel_attached_encoder(connector)->base);
 }
 
-static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
+static u32 g4x_infoframe_index(unsigned int type)
 {
 	switch (type) {
 	case HDMI_INFOFRAME_TYPE_AVI:
@@ -85,7 +85,7 @@  static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
 	}
 }
 
-static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
+static u32 g4x_infoframe_enable(unsigned int type)
 {
 	switch (type) {
 	case HDMI_INFOFRAME_TYPE_AVI:
@@ -100,9 +100,11 @@  static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
 	}
 }
 
-static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
+static u32 hsw_infoframe_enable(unsigned int type)
 {
 	switch (type) {
+	case DP_SDP_VSC:
+		return VIDEO_DIP_ENABLE_VSC_HSW;
 	case HDMI_INFOFRAME_TYPE_AVI:
 		return VIDEO_DIP_ENABLE_AVI_HSW;
 	case HDMI_INFOFRAME_TYPE_SPD:
@@ -118,10 +120,12 @@  static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
 static i915_reg_t
 hsw_dip_data_reg(struct drm_i915_private *dev_priv,
 		 enum transcoder cpu_transcoder,
-		 enum hdmi_infoframe_type type,
+		 unsigned int type,
 		 int i)
 {
 	switch (type) {
+	case DP_SDP_VSC:
+		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
 	case HDMI_INFOFRAME_TYPE_AVI:
 		return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder, i);
 	case HDMI_INFOFRAME_TYPE_SPD:
@@ -136,7 +140,7 @@  hsw_dip_data_reg(struct drm_i915_private *dev_priv,
 
 static void g4x_write_infoframe(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -191,7 +195,7 @@  static bool g4x_infoframe_enabled(struct drm_encoder *encoder,
 
 static void ibx_write_infoframe(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -251,7 +255,7 @@  static bool ibx_infoframe_enabled(struct drm_encoder *encoder,
 
 static void cpt_write_infoframe(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -309,7 +313,7 @@  static bool cpt_infoframe_enabled(struct drm_encoder *encoder,
 
 static void vlv_write_infoframe(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -368,7 +372,7 @@  static bool vlv_infoframe_enabled(struct drm_encoder *encoder,
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
 				const struct intel_crtc_state *crtc_state,
-				enum hdmi_infoframe_type type,
+				unsigned int type,
 				const void *frame, ssize_t len)
 {
 	const uint32_t *data = frame;
@@ -377,6 +381,8 @@  static void hsw_write_infoframe(struct drm_encoder *encoder,
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
 	i915_reg_t data_reg;
+	int data_size = type == DP_SDP_VSC ?
+		VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE;
 	int i;
 	u32 val = I915_READ(ctl_reg);
 
@@ -392,7 +398,7 @@  static void hsw_write_infoframe(struct drm_encoder *encoder,
 		data++;
 	}
 	/* Write every possible data byte to force correct ECC calculation. */
-	for (; i < VIDEO_DIP_DATA_SIZE; i += 4)
+	for (; i < data_size; i += 4)
 		I915_WRITE(hsw_dip_data_reg(dev_priv, cpu_transcoder,
 					    type, i >> 2), 0);
 	mmiowb();
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 93b177cc4cbf..6e3b430fccdc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -75,37 +75,6 @@  static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
 }
 
-static void intel_psr_write_vsc(struct intel_dp *intel_dp,
-				const struct edp_vsc_psr *vsc_psr)
-{
-	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
-	struct drm_device *dev = dig_port->base.base.dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
-	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
-	i915_reg_t ctl_reg = HSW_TVIDEO_DIP_CTL(cpu_transcoder);
-	uint32_t *data = (uint32_t *) vsc_psr;
-	unsigned int i;
-
-	/* As per BSPec (Pipe Video Data Island Packet), we need to disable
-	   the video DIP being updated before program video DIP data buffer
-	   registers for DIP being updated. */
-	I915_WRITE(ctl_reg, 0);
-	POSTING_READ(ctl_reg);
-
-	for (i = 0; i < sizeof(*vsc_psr); i += 4) {
-		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
-						   i >> 2), *data);
-		data++;
-	}
-	for (; i < VIDEO_DIP_VSC_DATA_SIZE; i += 4)
-		I915_WRITE(HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder,
-						   i >> 2), 0);
-
-	I915_WRITE(ctl_reg, VIDEO_DIP_ENABLE_VSC_HSW);
-	POSTING_READ(ctl_reg);
-}
-
 static void vlv_psr_setup_vsc(struct intel_dp *intel_dp,
 			      const struct intel_crtc_state *crtc_state)
 {
@@ -152,7 +121,8 @@  static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
 		psr_vsc.sdp_header.HB3 = 0x8;
 	}
 
-	intel_psr_write_vsc(intel_dp, &psr_vsc);
+	intel_dig_port->write_infoframe(&intel_dig_port->base.base, crtc_state,
+					DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc));
 }
 
 static void vlv_psr_enable_sink(struct intel_dp *intel_dp)