Re: [PATCH 11/25] drm/i915: Enable HDMI on ValleyView

[Shobhit Kumar <shobhit.kumar@intel.com>]

On Thursday 22 March 2012 02:32 AM, Daniel Vetter wrote:
> On Wed, Mar 21, 2012 at 12:48:32PM -0700, Jesse Barnes wrote:
>> From: Shobhit Kumar<shobhit.kumar@intel.com>
>>
>> HDMI register offsets are different in Valleyview. Add support for the
>> same.
>>
>> Signed-off-by: Beeresh G<beeresh.g@intel.com>
>> Signed-off-by: Shobhit Kumar<shobhit.kumar@intel.com>
>> Reviewed-by: Vijay Purushothaman<vijay.a.purushothaman@intel.com>
>> Reviewed-by: Jesse Barnes<jesse.barnes@intel.com>
>> Signed-off-by: Jesse Barnes<jbarnes@virtuousgeek.org>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h   |   16 +++++++++++++++
>>   drivers/gpu/drm/i915/intel_hdmi.c |   39 ++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 54 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index c187398..f04bfbf 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3452,6 +3452,21 @@
>>   #define TVIDEO_DIP_DATA(pipe) _PIPE(pipe, _VIDEO_DIP_DATA_A, _VIDEO_DIP_DATA_B)
>>   #define TVIDEO_DIP_GCP(pipe) _PIPE(pipe, _VIDEO_DIP_GCP_A, _VIDEO_DIP_GCP_B)
>>
>> +#define VLV_VIDEO_DIP_CTL_A		0x60220
>> +#define VLV_VIDEO_DIP_DATA_A		0x60208
>> +#define VLV_VIDEO_DIP_GDCP_PAYLOAD_A	0x60210
>> +
>> +#define VLV_VIDEO_DIP_CTL_B		0x61170
>> +#define VLV_VIDEO_DIP_DATA_B		0x61174
>> +#define VLV_VIDEO_DIP_GDCP_PAYLOAD_B	0x61178
>> +
>> +#define VLV_TVIDEO_DIP_CTL(pipe) \
>> +	 _PIPE(pipe, VLV_VIDEO_DIP_CTL_A, VLV_VIDEO_DIP_CTL_B)
>> +#define VLV_TVIDEO_DIP_DATA(pipe) \
>> +	 _PIPE(pipe, VLV_VIDEO_DIP_DATA_A, VLV_VIDEO_DIP_DATA_B)
>> +#define VLV_TVIDEO_DIP_GCP(pipe) \
>> +	_PIPE(pipe, VLV_VIDEO_DIP_GDCP_PAYLOAD_A, VLV_VIDEO_DIP_GDCP_PAYLOAD_B)
>> +
>>   #define _TRANS_HTOTAL_B          0xe1000
>>   #define _TRANS_HBLANK_B          0xe1004
>>   #define _TRANS_HSYNC_B           0xe1008
>> @@ -3672,6 +3687,7 @@
>>   #define  ADPA_CRT_HOTPLUG_FORCE_TRIGGER (1<<16)
>>
>>   /* or SDVOB */
>> +#define VLV_HDMIB 0x61140
>>   #define HDMIB   0xe1140
>>   #define  PORT_ENABLE    (1<<  31)
>>   #define  TRANSCODER(pipe)       ((pipe)<<  30)
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index cae3e5f..3f4a2d2 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -177,6 +177,37 @@ static void ironlake_write_infoframe(struct drm_encoder *encoder,
>>
>>   	I915_WRITE(reg, VIDEO_DIP_ENABLE | val | flags);
>>   }
>> +
>> +static void vlv_write_infoframe(struct drm_encoder *encoder,
>> +				     struct dip_infoframe *frame)
>> +{
>> +	uint32_t *data = (uint32_t *)frame;
>> +	struct drm_device *dev = encoder->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct drm_crtc *crtc = encoder->crtc;
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +	int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
>> +	unsigned i, len = DIP_HEADER_SIZE + frame->len;
>> +	u32 flags, val = I915_READ(reg);
>> +
>> +	intel_wait_for_vblank(dev, intel_crtc->pipe);
>> +
>> +	flags = intel_infoframe_index(frame);
>> +
>> +	val&= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
>> +
>> +	I915_WRITE(reg, VIDEO_DIP_ENABLE | val | flags);
>> +
>> +	for (i = 0; i<  len; i += 4) {
>> +		I915_WRITE(VLV_TVIDEO_DIP_DATA(intel_crtc->pipe), *data);
>> +		data++;
>> +	}
>> +
>> +	flags |= intel_infoframe_flags(frame);
>> +
>> +	I915_WRITE(reg, VIDEO_DIP_ENABLE | val | flags);
>> +}
>> +
>>   static void intel_set_infoframe(struct drm_encoder *encoder,
>>   				struct dip_infoframe *frame)
>>   {
>> @@ -522,10 +553,12 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
>>   	intel_encoder->crtc_mask = (1<<  0) | (1<<  1) | (1<<  2);
>>
>>   	/* Set up the DDC bus. */
>> +	/* For VLV SDVOB == HDMIB */
>>   	if (sdvox_reg == SDVOB) {
>>   		intel_encoder->clone_mask = (1<<  INTEL_HDMIB_CLONE_BIT);
>>   		intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
>>   		dev_priv->hotplug_supported_mask |= HDMIB_HOTPLUG_INT_STATUS;
>> +	/* For VLV SDVOC == HDMIC */
> I'm a bit confused about these two comments here. Afaict the code already
> works that way, not just on vlv. Can we just drop this hunk?
Yes we should drop this. Just couple of informational comments which I 
missed to remove.

Regards
Shobhit