[Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
Jose Abreu
Jose.Abreu at synopsys.com
Thu Mar 23 17:11:44 UTC 2017
Hi Shashank,
On 23-03-2017 16:43, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 3/23/2017 6:33 PM, Jose Abreu wrote:
>> Hi Shashank,
>>
>>
>> On 23-03-2017 16:08, Sharma, Shashank wrote:
>>> 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 at linux.intel.com>
>>>>>>> Cc: Jose Abreu <jose.abreu at synopsys.com>
>>>>>>> Cc: Andrzej Hajda <a.hajda at samsung.com>
>>>>>>> Cc: Alex Deucher <alexander.deucher at amd.com>
>>>>>>> Cc: Daniel Vetter <daniel.vetter at 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 at 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 ?
>> The DRM core propagates the mode to the chain of configuration
>> before reaching the bridge driver also, there is a callback
>> supplied by pdata (mode_valid) which can check if the mode is
>> valid. (see
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Eairlied_linux_tree_drivers_gpu_drm_bridge_synopsys_dw-2Dhdmi.c-3Fh-3Ddrm-2Dnext-23n1740&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zC1cWRy6RDWimQG2o866yYylV7c5h_U4aGWjVtnIHH0&s=DPZA3OgLWOO299hjy9dBDBK66kmUGfZArxKbeHNMKOc&e=
>> )
>>
>> Best regards,
>> Jose Miguel Abreu
>>
>>
>>> Regards
>>> Shashank
> Please correct me if my understanding is not right, but drivers
> call mode_valid() to prune/reject modes which they cant support.
> and they call drm_set_infoframe_from_videomode() function, when
> they are going ahead to the modeset with a mode.
> Why would a driver choose to do a modeset, which it could not
> support (shouldn't mode_valid have dropped it in atomic_check()
> or may be even edid_parsing time ?)
Yes, thats all correct :) I got a little out of topic here, sorry.
The thing I don't agree with is the adding of the parameter for
the drm_hdmi_avi_infoframe_from_display_mode(), because I think
its not enough. I will state why:
Picture this scenario:
1) EDID is read from HDMI 2.0 sink
2) The 2.0 modes are probed and as there is no check for
these modes (they are in the CEA table) they will be added to
mode list
3) The modes are passed to userspace. This is a problem
because userspace does not yet understand the new aspect ratios,
but lets imagine it understands
4) User selects an HDMI 2.0 mode
5) The drm core checks if mode is valid with driver, the
driver can say yes or no. Lets think it says yes because the
platform supports HDMI 2.0 modes.
6) Mode is committed but VIC will be set to zero according to
the patch
Now, according to 2.0 spec VIC shall be set when we are in 2.0
modes. I didn't do much inter-op with HDMI 2.0 receivers but I
can picture that some of them may just not display anything
because VIC and HDMI_VIC are not set. Though, from my experience,
generally the receiver is much more robust than the transmitter.
Even so, I think we should make sure that transmitter drivers are
fully compliant so that we can guarantee the support for less
robust receivers.
Best regards,
Jose Miguel Abreu
>
> Regards
> Shashank
More information about the Intel-gfx
mailing list