On 04.12.2018 20:02, Ville Syrjälä wrote: > On Tue, Dec 04, 2018 at 08:03:53AM +0100, Andrzej Hajda wrote: >> 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. > Like I said, The HDMI 1.4 method is used even with HDMI 2.0 sinks unless > some feature gets used which can't be signalled via the HDMI 1.4 vendor > specific infoframe. Do you mean that 4K VICs 95, 94, 93, 98 defined in CEA-861-F are not used at all for non-3d video in HDMI 2.0? Chapter 10.1 of HDMI2.0 spec says clearly: > When transmitting any additional Video Format for which a VIC value > has been defined in > CEA-861-F tables 1, 2, and 3, an HDMI Source shall set the VIC field > to the Video Code for > that format. It contradicts your statement, or am I missing something? > >> >> So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems >> risky. > That is not what I was proposing. Maybe I have misread your statement: On 21.11.2018 12:51, Ville Syrjälä wrote: > 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. I do not know what it could mean other that allowing sending VICs unknown to sink? Maybe better would be just to wait for the patch to avoid misunderstandings. Regards Andrzej > >> >> 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