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@xxxxxxxxxx>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
>>>> ---
>>>> 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.
Best regards,
Jose Miguel Abreu
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
