Hi
2013/1/31 Jani Nikula <jani.nikula at linux.intel.com>:
> On Wed, 30 Jan 2013, Rodrigo Vivi <rodrigo.vivi at gmail.com> wrote:
>> While old platforms had 3 transcoders and 3 pipes (1:1), HSW has 4 transcoders and 3 pipes. To avoid future mistakes transcoders must be used here instead of pipes even though it is working right now by coincidence.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
>> ---
>> drivers/gpu/drm/i915/i915_reg.h | 16 ++++++++--------
>> drivers/gpu/drm/i915/intel_hdmi.c | 12 ++++++------
>> 2 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 2521617..7bb3134 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3742,14 +3742,14 @@
>> #define HSW_VIDEO_DIP_VSC_ECC_B 0x61344
>> #define HSW_VIDEO_DIP_GCP_B 0x61210
>>
>> -#define HSW_TVIDEO_DIP_CTL(pipe) \
>> - _PIPE(pipe, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
>> -#define HSW_TVIDEO_DIP_AVI_DATA(pipe) \
>> - _PIPE(pipe, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
>> -#define HSW_TVIDEO_DIP_SPD_DATA(pipe) \
>> - _PIPE(pipe, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
>> -#define HSW_TVIDEO_DIP_GCP(pipe) \
>> - _PIPE(pipe, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
>> +#define HSW_TVIDEO_DIP_CTL(trans) \
>> + _TRANSCODER(trans, HSW_VIDEO_DIP_CTL_A, HSW_VIDEO_DIP_CTL_B)
>> +#define HSW_TVIDEO_DIP_AVI_DATA(trans) \
>> + _TRANSCODER(trans, HSW_VIDEO_DIP_AVI_DATA_A, HSW_VIDEO_DIP_AVI_DATA_B)
>> +#define HSW_TVIDEO_DIP_SPD_DATA(trans) \
>> + _TRANSCODER(trans, HSW_VIDEO_DIP_SPD_DATA_A, HSW_VIDEO_DIP_SPD_DATA_B)
>> +#define HSW_TVIDEO_DIP_GCP(trans) \
>> + _TRANSCODER(trans, HSW_VIDEO_DIP_GCP_A, HSW_VIDEO_DIP_GCP_B)
>
> Okay, I'm confused. The patch does make sense, but bspec doesn't... the
> spec says the above HSW_TVIDEO_DIP_* registers (in the 0x6xxxx range)
> are per pipe, and the TVIDEO_DIP_* registers (in the 0xexxxx range) are
> per transcoder. Care to enlighten me here...?
Yes. TVIDEO_DIP_* registers don't exist on LPT (which is used by
Haswell), they exist on IBX/CPT/PPT. On these older platforms, pipe
and cpu_transcoder are the same thing, so the value is always the
same. Only after Haswell the old "cpu pipe" was split into "cpu pipe"
and "cpu transcoder". These DIP registers migrated from the PCH to the
CPU on Haswell.
>
> Also, outside of this patch, have we reviewed if there's always a
> register for TRANSCODER_EDP where _TRANSCODER() macro is used?
On Haswell it should exist unless it's a register that is never used
on DP code, so its TRANSCODER_EDP version will never be used.
>
> BR,
> Jani.
>
>
>>
>> #define _TRANS_HTOTAL_B 0xe1000
>> #define _TRANS_HBLANK_B 0xe1004
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index d53b731..ab95e05 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -120,13 +120,13 @@ static u32 hsw_infoframe_enable(struct dip_infoframe *frame)
>> }
>> }
>>
>> -static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum pipe pipe)
>> +static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame, enum transcoder cpu_transcoder)
>> {
>> switch (frame->type) {
>> case DIP_TYPE_AVI:
>> - return HSW_TVIDEO_DIP_AVI_DATA(pipe);
>> + return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
>> case DIP_TYPE_SPD:
>> - return HSW_TVIDEO_DIP_SPD_DATA(pipe);
>> + return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
>> default:
>> DRM_DEBUG_DRIVER("unknown info frame type %d\n", frame->type);
>> return 0;
>> @@ -293,8 +293,8 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>> struct drm_device *dev = encoder->dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>> - u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
>> - u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->pipe);
>> + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
>> + u32 data_reg = hsw_infoframe_data_reg(frame, intel_crtc->cpu_transcoder);
>> unsigned int i, len = DIP_HEADER_SIZE + frame->len;
>> u32 val = I915_READ(ctl_reg);
>>
>> @@ -566,7 +566,7 @@ static void hsw_set_infoframes(struct drm_encoder *encoder,
>> struct drm_i915_private *dev_priv = encoder->dev->dev_private;
>> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
>> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>> - u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->pipe);
>> + u32 reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
>> u32 val = I915_READ(reg);
>>
>> assert_hdmi_port_disabled(intel_hdmi);
>> --
>> 1.7.11.7
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Paulo Zanoni
