On 03.12.2018 22:48, Ville Syrjälä wrote:
> On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
>> Quite late, hopefully not too late.
>>
>>
>> On 21.11.2018 12:51, Ville Syrjälä wrote:
>>> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
>>>>> return;
>>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> index a6e8f4591e63..0cc293a6ac24 100644
>>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
>>>>> int ret;
>>>>>
>>>>> ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>>>>> - mode,
>>>>> - true);
>>>>> + NULL, mode);
>>>>> if (ctx->use_packed_pixel)
>>>>> frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> index 64c3cf027518..88b720b63126 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> @@ -1344,7 +1344,8 @@ 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, false);
>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame,
>>>>> + &hdmi->connector, mode);
>>>>>
>>>>> if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>>> index b506e3622b08..501ac05ba7da 100644
>>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
>>>>> }
>>>>> EXPORT_SYMBOL(drm_set_preferred_mode);
>>>>>
>>>>> +static bool is_hdmi2_sink(struct drm_connector *connector)
>>>> You're usually known for adding const all around, why not const pointer
>>>> here and in all the other drm_* functions that call this?
>>> My current approach is to constify states/fbs/etc. but not so much
>>> crtcs/connectors/etc. Too much const can sometimes get in the way
>>> of things requiring that you remove the const later. But I guess
>>> in this case the const shouldn't really get in the way of anything
>>> because these are pretty much supposed to be pure functions.
>>>
>>>>> +{
>>>>> + /*
>>>>> + * FIXME: sil-sii8620 doesn't have a connector around when
>>>>> + * we need one, so we have to be prepared for a NULL connector.
>>>>> + */
>>>>> + if (!connector)
>>>>> + return false;
>>>> This actually changes the is_hdmi2_sink value for sil-sii8620.
>>> Hmm. No idea why they would have set that to true when everyone else is
>>> passing false.
>>
>> Because false does not work :) More precisely MHLv3 (used in Sii8620)
>> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
>>
>> Unfortunately I have no access to MHL specs, but my experiments and
>> vendor drivers strongly suggests it is done this way.
>>
>> This is important in case of 4K modes which are handled differently by
>> HDMI 1.4 and HDMI2.0.
> HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
> the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
> switch over to the HDMI 2.0 specific signalling.
The difference is in infoframes:
HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI.
HDMI 2.0 sets AVI infoframe to non zero VICs introduced by
HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d
is in use.
So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems
risky.
Regards
Andrzej
>
>> The pipeline looks like (in parenthesis HDMI version on the stream):
>>
>> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
>>
>>
>>> I guess I can change this to true to not change it. IIRC
>>> that was the only driver that didn't have a connector around.
>>>
>>> That said, I was actually thinking of removing this hdmi2 vs. not
>>> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
>>> "just in case", but we already have a similar issue with earlier
>>> cea-861 revisions and haven't seen any bugs where an older monitor
>>> would get confused by a VIC not yet defined when the monitor was
>>> produced.
>>>
>> Are you sure this is a good idea? As I said earlier 4K modes are present
>> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
>> only matter of unknown VIC in AVIF.
>>
>>
>> Regards
>>
>> Andrzej
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
