[Intel-gfx] [PATCH v2 3/6] drm/edid: detect SCDC support in HF-VSDB

[Sharma, Shashank]

Regards

Shashank


On 2/8/2017 5:06 PM, Jose Abreu wrote:
> Hi,
>
>
>
> On 07-02-2017 16:36, Thierry Reding wrote:
>> On Tue, Feb 07, 2017 at 09:43:15PM +0530, Sharma, Shashank wrote:
>>> Regards
>>>
>>> Shashank
>>>
>>>
>>> On 2/7/2017 4:31 PM, Jose Abreu wrote:
>>>> Hi Shashank,
>>>>
>>>>
>>>>
>>>> On 06-02-2017 13:59, Shashank Sharma wrote:
>>>>> This patch does following:
>>>>> - Adds a new structure (drm_hdmi_info) in drm_display_info.
>>>>>     This structure will be used to save and indicate if sink
>>>>>     supports advanced HDMI 2.0 features
>>>>> - Adds another structure drm_scdc within drm_hdmi_info, to
>>>>>     reflect scdc support and capabilities in connected HDMI 2.0 sink.
>>>>> - Checks the HF-VSDB block for presence of SCDC, and marks it
>>>>>     in scdc structure
>>>>> - If SCDC is present, checks if sink is capable of generating
>>>>>     SCDC read request, and marks it in scdc structure.
>>>>>
>>>>> V2: Addressed review comments
>>>>> Thierry:
>>>>> - Fix typos in commit message and make abbreviation consistent
>>>>>     across the commit message.
>>>>> - Change structure object name from hdmi_info -> hdmi
>>>>> - Fix typos and abbreviations in description of structure drm_hdmi_info
>>>>>     end the description with a full stop.
>>>>> - Create a structure drm_scdc, and keep all information related to SCDC
>>>>>     register set (supported, read request supported) etc in it.
>>>>>
>>>>> Ville:
>>>>> - Change rr -> read_request
>>>>> - Call drm_detect_scrambling function drm_parse_hf_vsdb so that all
>>>>>     of HF-VSDB parsing can be kept in same function, in incremental
>>>>>     patches.
>>>>>
>>>>> Reviewed-by: Thierry Reding <treding at nvidia.com>
>>>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/drm_edid.c  | 14 ++++++++++++++
>>>>>    include/drm/drm_connector.h | 33 +++++++++++++++++++++++++++++++++
>>>>>    2 files changed, 47 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>>> index 96d3e47..a487b80 100644
>>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>>> @@ -3802,6 +3802,18 @@ enum hdmi_quantization_range
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_default_rgb_quant_range);
>>>>> +static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
>>>>> +				 const u8 *hf_vsdb)
>>>>> +{
>>>>> +	struct drm_hdmi_info *hdmi = &connector->display_info.hdmi;
>>>>> +
>>>>> +	if (hf_vsdb[6] & 0x80) {
>>>> BIT(7) ?
>>> Yes, SCDC_present bit is bit 7, byte 6 in HF-VSDB. Am I missing something ?
>>>>> +		hdmi->scdc.supported = true;
>>>>> +		if (hf_vsdb[6] & 0x40)
>>>> BIT(6) ?
>>> Yes, RR_Capable bit is bit 6, byte 6 in HF-VSDB.
>> I think what Jose was trying to say is that you should be using BIT(7)
>> instead of 0x80 and BIT(6) instead of 0x40. That said, I think either is
>> fine, but perhaps another idea would be to define macros for these. I
>> know that most (all?) of the EDID parsing code uses literals, so this is
>> consistent with existing code. Also usually code will be like:
>>
>> 	if (hf_vsdb[X] & 0xYZ)
>> 		foo_supported = true;
>>
>> So the meaning of the bit is easy to read from the context. I think
>> literals are fine in this case.
>>
>> Thierry
> Thats exactly what I meant :) I think with BIT(x) the code is
> easier to read (my hex skills are not very good :)). Anyway, if
> the remaining code uses literals then maybe its better to keep
> consistency.
Thanks, then we will keep this code, as it is.
- Shashank
> Best regards,
> Jose Miguel Abreu
>