Regards Shashank On 6/13/2016 5:57 PM, Ville Syrjälä wrote:
Ok, This patch generally looks good, but I am not sure I am the right guy to check this patch further, so I will pass it from here. We might need another opinion on this.On Mon, Jun 13, 2016 at 05:39:47PM +0530, Sharma, Shashank wrote:Regards Shashank On 6/3/2016 1:25 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote:From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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. Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_hdmi.c | 26 +++++++++++++++---------- drivers/gpu/drm/i915/intel_psr.c | 41 ++++++++------------------------------- 3 files changed, 25 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4c8451e3d8f1..5dcaa14ff90d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -900,7 +900,7 @@ struct intel_digital_port { /* for communication with audio component; protected by av_mutex */ const struct drm_connector *audio_connector; void (*write_infoframe)(struct drm_encoder *encoder, - 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 637b17baf798..600a58210450 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -68,7 +68,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: @@ -83,7 +83,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: @@ -98,9 +98,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:Why do we need a new field like this ? Would it make sense to set the type itself as "HDMI_INFOFRAME_TYPE_AVI" ?We want to enable/write the VSC, not the AVI.
+ return VIDEO_DIP_ENABLE_VSC_HSW; case HDMI_INFOFRAME_TYPE_AVI: return VIDEO_DIP_ENABLE_AVI_HSW; case HDMI_INFOFRAME_TYPE_SPD: @@ -116,10 +118,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: @@ -133,7 +137,7 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv, } static void g4x_write_infoframe(struct drm_encoder *encoder, - enum hdmi_infoframe_type type, + unsigned int type, const void *frame, ssize_t len) { const uint32_t *data = frame; @@ -187,7 +191,7 @@ static bool g4x_infoframe_enabled(struct drm_encoder *encoder, } static void ibx_write_infoframe(struct drm_encoder *encoder, - enum hdmi_infoframe_type type, + unsigned int type, const void *frame, ssize_t len) { const uint32_t *data = frame; @@ -246,7 +250,7 @@ static bool ibx_infoframe_enabled(struct drm_encoder *encoder, } static void cpt_write_infoframe(struct drm_encoder *encoder, - enum hdmi_infoframe_type type, + unsigned int type, const void *frame, ssize_t len) { const uint32_t *data = frame; @@ -303,7 +307,7 @@ static bool cpt_infoframe_enabled(struct drm_encoder *encoder, } static void vlv_write_infoframe(struct drm_encoder *encoder, - enum hdmi_infoframe_type type, + unsigned int type, const void *frame, ssize_t len) { const uint32_t *data = frame; @@ -361,7 +365,7 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder, } static void hsw_write_infoframe(struct drm_encoder *encoder, - enum hdmi_infoframe_type type, + unsigned int type, const void *frame, ssize_t len) { const uint32_t *data = frame; @@ -371,6 +375,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder, enum transcoder cpu_transcoder = intel_crtc->config->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 ?Ok, I think may be this is the reason why we need a separate filed for DP_SDP_VSC, is it so ?No, we need to regarless of the size of the buffer.- Shashank+ VIDEO_DIP_VSC_DATA_SIZE : VIDEO_DIP_DATA_SIZE; int i; u32 val = I915_READ(ctl_reg); @@ -386,7 +392,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 29a09bf6bd18..904994dd1c9e 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -72,37 +72,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 = dev->dev_private; - 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) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); @@ -121,6 +90,7 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp) static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp) { + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct edp_vsc_psr psr_vsc; /* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */ @@ -129,11 +99,14 @@ static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp) psr_vsc.sdp_header.HB1 = 0x7; psr_vsc.sdp_header.HB2 = 0x3; psr_vsc.sdp_header.HB3 = 0xb; - intel_psr_write_vsc(intel_dp, &psr_vsc); + + intel_dig_port->write_infoframe(&intel_dig_port->base.base, + DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc)); } static void hsw_psr_setup_vsc(struct intel_dp *intel_dp) { + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct edp_vsc_psr psr_vsc; /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */ @@ -142,7 +115,9 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp) psr_vsc.sdp_header.HB1 = 0x7; psr_vsc.sdp_header.HB2 = 0x2; psr_vsc.sdp_header.HB3 = 0x8; - intel_psr_write_vsc(intel_dp, &psr_vsc); + + intel_dig_port->write_infoframe(&intel_dig_port->base.base, + DP_SDP_VSC, &psr_vsc, sizeof(psr_vsc)); } static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
