[Intel-gfx] [PATCH v3 1/3] drm: Create new structure for HDMI info

[Sharma, Shashank]

Regards

Shashank


On 12/22/2016 5:26 PM, Ville Syrjälä wrote:
> On Thu, Dec 22, 2016 at 10:02:26AM +0000, Jose Abreu wrote:
>> Hi Shashank,
>>
>>
>> On 21-12-2016 15:29, Shashank Sharma wrote:
>>
>> [snip]
>>
>>> +
>>> +	/**
>>> +	 * @edid_yuv420_dc_modes: bpc for deep color yuv420 encoding.
>>> +	 * various sinks can support 10/12/16 bit per channel deep
>>> +	 * color encoding. edid_yuv420_dc_modes = 0 means sink doesn't
>>> +	 * support deep color yuv420 encoding.
>>> +	 */
>>> +	u8 edid_yuv420_dc_modes;
>>> +
>>> +
>>> +#define DRM_HFVSDB_SCDC_SUPPORT	(1<<7)
>>> +#define DRM_HFVSDB_SCDC_RR_CAP		(1<<6)
>>> +#define DRM_HFVSDB_SCRAMBLING		(1<<3)
>>> +#define DRM_HFVSDB_INDEPENDENT_VIEW	(1<<2)
>>> +#define DRM_HFVSDB_DUAL_VIEW		(1<<1)
>>> +#define DRM_HFVSDB_3D_OSD		(1<<0)
>>> +
>>> +	/**
>>> +	 * @scdc_supported: Sink supports SCDC functionality.
>>> +	 */
>>> +	bool scdc_supported;
>>> +
>>> +	/**
>>> +	 * @scdc_rr_cap: Sink has SCDC read request capability.
>>> +	 */
>>> +	bool scdc_rr_cap;
>>> +
>>> +	/**
>>> +	 * @scrambling: Sync supports scrambling for <=340 Mcsc TMDS
>>> +	 * char rates. Above 340 Mcsc rates, scrambling is always reqd.
>>> +	 */
>>> +	bool scrambling;
>>> +
>>> +	/**
>>> +	 * @independent_view_3d: Sink supports 3d independent view signaling
>>> +	 * in HF-VSIF.
>>> +	 */
>>> +	bool independent_view_3d;
>>> +
>>> +	/**
>>> +	 * @dual_view_3d: Sink supports 3d dual view signaling in HF-VSIF.
>>> +	 */
>>> +	bool dual_view_3d;
>>> +
>>> +	/**
>>> +	 * @osd_disparity_3d: Sink supports 3d osd disparity indication
>>> +	 * in HF-VSIF.
>>> +	 */
>>> +	bool osd_disparity_3d;
>>> +};
>> [snip]
>>
>> I thought we agreed in only adding these fields
>> (edid_yuv420_dc_modes, scdc_supported, scdc_rr_cap, scrambling,
>> independent_view_3d, dual_view_3d, osd_disparity_3d) in patch 2.
>> I think it makes sense because you are only using them after that
>> patch. Though, I would like to hear more comments about this as I
>> am quite new to dri-devel.
> Generally you shouldn't add stuff you don't use. In this seires nothing
> actually uses any of of this stuff, so I don't think we should add
> any of it.
>
> The only piece of information actually used is the max TMDS clock, so
> parsing that does make sense. But I think that might be buggy as the
> patch will go ahead and parse both the old and new HDMI data blocks.
> IIRC the spec did have some words about which one should be used in
> which case. I don't think I spotted anything matching that in these
> patches.
>
If I understood the spec right, H14B VSDB block should contain the 
max_tmds_clock value to be reflected for tmds_clock_rates <340Mhz,
But if the sink supports tmds_rates above 340Mhz, it should set this 
field in hf-vsdb.
Also, this field allows sinks to support higher color depths to lower 
resolutions, than it can support to higher resolutions.

In this case, if this byte is hf-vsdb is set, max tmds clock should be 
picked from this block.

Regards
Shashank

Regards
Shashank