Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames

["Sharma, Shashank" <shashank.sharma@intel.com>]

Regards

Shashank


On 3/23/2017 5:57 PM, Jose Abreu wrote:
> Hi Ville,
>
>
> On 23-03-2017 15:45, Ville Syrjälä wrote:
>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>> Hi Shashank,
>>>
>>>
>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
>>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>>> extended to (VIC 1-107).
>>>>
>>>> This patch adds a bool input variable, which indicates if the connected
>>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>
>>>> This patch touches all drm drivers, who are callers of this function
>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>
>>>> In case of I915 driver, this patch checks the connector->display_info
>>>> to check if the connected display is HDMI 2.0.
>>>>
>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>
>>>> PS: This patch touches a few lines in few files, which were
>>>> already above 80 char, so checkpatch gives 80 char warning again.
>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>> - gpu/drm/i915/intel_sdvo.c
>>>>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>>>>   drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>>>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>>>>   drivers/gpu/drm/bridge/analogix-anx78xx.c |  3 ++-
>>>>   drivers/gpu/drm/bridge/sii902x.c          |  2 +-
>>>>   drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
>>>>   drivers/gpu/drm/drm_edid.c                | 12 +++++++++++-
>>>>   drivers/gpu/drm/exynos/exynos_hdmi.c      |  2 +-
>>>>   drivers/gpu/drm/i2c/tda998x_drv.c         |  2 +-
>>>>   drivers/gpu/drm/i915/intel_hdmi.c         |  5 ++++-
>>>>   drivers/gpu/drm/i915/intel_sdvo.c         |  3 ++-
>>>>   drivers/gpu/drm/mediatek/mtk_hdmi.c       |  2 +-
>>>>   drivers/gpu/drm/omapdrm/omap_encoder.c    |  3 ++-
>>>>   drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>>>>   drivers/gpu/drm/rockchip/inno_hdmi.c      |  2 +-
>>>>   drivers/gpu/drm/sti/sti_hdmi.c            |  2 +-
>>>>   drivers/gpu/drm/tegra/hdmi.c              |  2 +-
>>>>   drivers/gpu/drm/tegra/sor.c               |  2 +-
>>>>   drivers/gpu/drm/vc4/vc4_hdmi.c            |  2 +-
>>>>   drivers/gpu/drm/zte/zx_hdmi.c             |  2 +-
>>>>   include/drm/drm_edid.h                    |  3 ++-
>>>>   21 files changed, 38 insertions(+), 21 deletions(-)
>>>>
>>> [snip]
>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> index af93f7a..5ff2886 100644
>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>   	u8 val;
>>>>   
>>>>   	/* Initialise info frame from DRM mode */
>>>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>>>   
>>>>   	if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>>   		frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>
>>> dw-hdmi controller has full support for HDMI 2.0 features. It all
>>> depends on the platform it is integrated.
>>>
>>> I think adding a parameter to
>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>> because of this case: A bridge can have support for HDMI 2.0
>>> features but the platform may limit this support. I guess it can
>>> happen in other drivers too.
>> Your driver is in full control of what gets passed here. So I don't see
>> why that would be a problem.
>>
>> Also this doesn't really have anything to do with the capabilities of
>> the source. All we want to make sure is that we don't send a VIC the
>> sink will not understand.
>>
> But the driver is not aware of the platform limitations, its
> generic to the controller only. We could add a field in pdata
> which tells if platform is HDMI 2.0+ but what about other bridge
> drivers or HDMI drivers? They will have to replicate the same
> thing also.
>
> Best regards,
> Jose Miguel Abreu
I think the driver would be aware of the platform's capabilities, isn't 
it ?
Else how would it even decide which mode to allow, and which to reject ?

Regards
Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel