[Intel-gfx] [PATCH 4/6] drm: scrambling support in drm layer

[Sharma, Shashank]

Regards

Shashank


On 2/2/2017 3:58 PM, Ville Syrjälä wrote:
> On Thu, Feb 02, 2017 at 03:46:55PM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 2/2/2017 3:21 PM, Ville Syrjälä wrote:
>>> On Thu, Feb 02, 2017 at 11:18:51AM +0530, Sharma, Shashank wrote:
>>>> Regards
>>>>
>>>> Shashank
>>>>
>>>>
>>>> On 2/1/2017 10:02 PM, Ville Syrjälä wrote:
>>>>> On Wed, Feb 01, 2017 at 06:14:39PM +0530, Shashank Sharma wrote:
>>>>>> HDMI 2.0 spec mandates scrambling for modes with pixel clock higher
>>>>>> than 340Mhz. This patch adds few new functions in drm layer for
>>>>>> core drivers to enable/disable scrambling.
>>>>>>
>>>>>> This patch adds:
>>>>>> - A function to detect scrambling support parsing HF-VSDB
>>>>>> - A function to check scrambling status runtime using SCDC read.
>>>>>> - Two functions to enable/disable scrambling using SCDC read/write.
>>>>>> - Few new bools to reflect scrambling support and status.
>>>>>>
>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma at intel.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/drm_edid.c  | 131 +++++++++++++++++++++++++++++++++++++++++++-
>>>>>>     include/drm/drm_connector.h |  24 ++++++++
>>>>>>     include/drm/drm_edid.h      |   6 +-
>>>>>>     3 files changed, 159 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>>>> index 37902e5..f0d940a 100644
>>>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>>>> @@ -37,6 +37,7 @@
>>>>>>     #include <drm/drm_edid.h>
>>>>>>     #include <drm/drm_encoder.h>
>>>>>>     #include <drm/drm_displayid.h>
>>>>>> +#include <drm/drm_scdc_helper.h>
>>>>>>     
>>>>>>     #define version_greater(edid, maj, min) \
>>>>>>     	(((edid)->version > (maj)) || \
>>>>>> @@ -3814,6 +3815,132 @@ static void drm_detect_hdmi_scdc(struct drm_connector *connector,
>>>>>>     	}
>>>>>>     }
>>>>>>     
>>>>>> +static void drm_detect_hdmi_scrambling(struct drm_connector *connector,
>>>>>> +				 const u8 *hf_vsdb)
>>>>> That names seems off. Should probably be drm_parse_hdmi_forum_vsdb() or
>>>>> something.
>>>> Actually, unlike the last patch set, we are not parsing the whole
>>>> hf_vsdb, but parsing it only for
>>>> scrambling status byte (hf_vsdb[5]). But may be I can make it
>>>> drm_detect_scrambling_from_hfvsdb
>>>> ot something similar. We will have more hf_vsdb parsing for 3d flags,
>>>> yuv420_deep_color etc.
>>> Well, so far I'm not seeing much point in splitting it up. So I'd stuff
>>> it all into one place, for now at least.
>> I had published a patch just doing as you suggested which was parsing
>> complete hf-vsdb in one shot, and adding info in
>> hdmi_info structure, to accommodate it.
>> (Patch https://patchwork.freedesktop.org/patch/128963/ and
>> https://patchwork.freedesktop.org/patch/128962/)
>>
>> But you gave a review comment not to add anything which is not being
>> used in the patch series:
>> https://patchwork.freedesktop.org/patch/128962/
> That's doesn't require the things that are left to be split into
> separate functions.
So please let me know if my understanding is correct.
- Name the function as *_parse_hfvsdb only
- Call it while parsing the EDID extension blocks (the same place)
- Keep on adding parsing of the HF-VSDB blocks, in the same function, in 
an incremental way, as we add patches for them.
In this way the function won't be split into multiple small functions, 
but all the parsing will be done in one function.

- Shashank
>> That was the only reason I have split hf-vsdb parsing patch into 3 parts
>> - parse SCDC and scrambling info (here)
>> - with YUV420 deep color ( upcoming )
>> - with 3d handling (upcoming)
>>
>> So I am confused, split or no split :-) ?
>>
>> - Shashank