Hi Ville,
On Tuesday 04 Jul 2017 15:44:02 Ville Syrjälä wrote:
> On Tue, Jul 04, 2017 at 01:56:07PM +0200, Andrzej Hajda wrote:
> > On 03.07.2017 21:19, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>
> >> Appedix F of HDMI 2.0 says that some HDMI sink may fail to switch from
> >> 3D to 2D mode in a timely fashion if the source simply stops sending the
> >> HDMI infoframe. The suggested workaround is to keep sending the
> >> infoframe even when strictly not necessary (ie. no VIC and no S3D).
> >> HDMI 1.4 does allow for this behaviour, stating that sending the
> >> infoframe is optional in this case.
> >
> > My impression from the specs is that it should be done only after
> > switching from 3d to 2d mode.
> > In such case we just need to remember previous mode, if it was 3d, empty
> > VSIF infoframe should be still generated for 2seconds.
> > No need to do guesses from EDID.
> > Am I right, or just missing something?
>
> This code has no idea about any 3D->2D transitions, trying to make it
> do that would just result in a lot of complexity. Much easier to just
> always send the infoframe.
>
> >> The infoframe was first specified in HDMI 1.4, so in theory sinks
> >> predating that may not appreciate us sending an uknown infoframe
> >> their way. To avoid regressions let's try to determine if the sink
> >> supports the infoframe or not. Unfortunately there's no direct way
> >> to do that, so instead we'll just check if we managed to parse any
> >> HDMI 1.4 4k or stereo modes from the EDID, and if so we assume the
> >> sink will accept the infoframe. Also if the EDID contains the HDMI
> >> 2.0 HDMI Forum VSDB we can assume the sink is prepared to receive
> >> the infoframe.
> >> Cc: Archit Taneja <architt@xxxxxxxxxxxxxx>
> >> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> >> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> >> Cc: Inki Dae <inki.dae@xxxxxxxxxxx>
> >> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> >> Cc: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
> >> Cc: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> >> Cc: CK Hu <ck.hu@xxxxxxxxxxxx>
> >> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> >> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
> >> Cc: Mark Yao <mark.yao@xxxxxxxxxxxxxx>
> >> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>
> >> Cc: Vincent Abriou <vincent.abriou@xxxxxx>
> >> Cc: Shawn Guo <shawnguo@xxxxxxxxxx>
> >> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> ---
> >>
> >> drivers/gpu/drm/bridge/sil-sii8620.c | 3 ++-
> >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +++-
> >> drivers/gpu/drm/drm_edid.c | 32 ++++++++++++++++++------
> >> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> >> drivers/gpu/drm/i915/intel_hdmi.c | 14 ++++++++------
> >> drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 ++-
> >> drivers/gpu/drm/nouveau/nv50_display.c | 3 ++-
> >> drivers/gpu/drm/rockchip/inno_hdmi.c | 1 +
> >> drivers/gpu/drm/sti/sti_hdmi.c | 4 +++-
> >> drivers/gpu/drm/zte/zx_hdmi.c | 1 +
> >> include/drm/drm_connector.h | 5 +++++
> >> include/drm/drm_edid.h | 1 +
> >> 12 files changed, 55 insertions(+), 18 deletions(-)
[snip]
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 2e55599816aa..c061dd5d25c0 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
[snip]
> >> @@ -4452,6 +4458,7 @@ s3d_structure_from_display_mode(const struct
> >> drm_display_mode *mode)> >
> >> * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI
> >> infoframe with
> >> * data from a DRM display mode
> >> * @frame: HDMI vendor infoframe
> >> + * @connector: the connector
> >> * @mode: DRM display mode
> >> *
> >> * Note that there's is a need to send HDMI vendor infoframes only when
> >> using a
> >> @@ -4462,8 +4469,15 @@ s3d_structure_from_display_mode(const struct
> >> drm_display_mode *mode)
> >> */
> >>
> >> int
> >> drm_hdmi_vendor_infoframe_from_display_mode(struct
> >> hdmi_vendor_infoframe *frame,
> >> + struct drm_connector *connector,
> >> const struct drm_display_mode
> >> *mode)
> >> {
> >> + /*
> >> + * FIXME: sil-sii8620 doesn't have a connector around when
> >> + * we need one, so we have to be prepared for a NULL connector.
> >> + */
> >> + bool has_hdmi_infoframe = connector ?
> >> + &connector->display_info.has_hdmi_infoframe : NULL;
> >
> > I wonder if requiring connector is a good idea, I can imagine that this
> > function can be necessary in pure drm_encoder or non-terminal drm_bridge.
>
> No decent way around it, unless we want to risk sending the infoframe to
> pre HDMI 1.4 sinks. Sounds like you have some kind of layering problem
> if you can't get at the connector when you call this.
I think you have a layering violation if you assume that there's a connector
after the HDMI encoder :-) What if the encoder's output is connected to, let's
say, an HDMI to DSI bridge ? There's no connector in that case.
> > > int err;
> > > u32 s3d_flags;
> > > u8 vic;
[snip]
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
