On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote: > On 03.12.2018 22:38, Ville Syrjälä wrote: > > On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote: > >> On 21.11.2018 19:19, Laurent Pinchart wrote: > >>> Hi Ville, > >>> > >>> Thank you for the patch. > >>> > >>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote: > >>>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >>>> > >>>> Make life easier for drivers by simply passing the connector > >>>> to drm_hdmi_avi_infoframe_from_display_mode() and > >>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't > >>>> need to worry about is_hdmi2_sink mess. > >>> While this is good for display controller drivers, the change isn't great for > >>> bridge drivers. Down the road we're looking at moving connector support out of > >>> the bridge drivers. Adding an additional dependency to connectors in the > >>> bridges will make that more difficult. Ideally bridges should retrieve the > >>> information from their sink, regardless of whether it is a connector or > >>> another bridge. > >> > >> I agree with it, and case of sii8620 shows that there are cases where > >> bridge has no direct access to the connector. > > It's just a matter of plumbing it through. > > > What do you mean exactly? void bridge_foo(... + ,struct drm_connector *connector); > > > > > >> On the other side, since you are passing connector to > >> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode > >> parameter and rename the function to > >> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and > >> mode set on the connector differs? > > Connectors don't have a mode. > > > As they are passing video stream they should have it, even if not > directly, for example: > > connector->state->crtc->mode That's not really how atomic works. One shouldn't go digging through the obj->state pointers when we're not holding the relevant locks anymore. The atomic way would be to pass either both crtc state and connector state, or drm_atomic_state + crtc/connector. > > In moment of creating infoframe it should be set properly. > > > Regards > > Andrzej > > > > > >> > >> Regards > >> > >> Andrzej > >> > >> > >>> Please see below for an additional comment. > >>> > >>>> Cc: Alex Deucher <alexander.deucher@xxxxxxx> > >>>> Cc: "Christian König" <christian.koenig@xxxxxxx> > >>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@xxxxxxx> > >>>> 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: Russell King <linux@xxxxxxxxxxxxxxx> > >>>> Cc: CK Hu <ck.hu@xxxxxxxxxxxx> > >>>> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > >>>> Cc: Rob Clark <robdclark@xxxxxxxxx> > >>>> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx> > >>>> Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxx> > >>>> Cc: Sandy Huang <hjc@xxxxxxxxxxxxxx> > >>>> Cc: "Heiko Stübner" <heiko@xxxxxxxxx> > >>>> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx> > >>>> Cc: Vincent Abriou <vincent.abriou@xxxxxx> > >>>> Cc: Thierry Reding <thierry.reding@xxxxxxxxx> > >>>> Cc: Eric Anholt <eric@xxxxxxxxxx> > >>>> Cc: Shawn Guo <shawnguo@xxxxxxxxxx> > >>>> Cc: Ilia Mirkin <imirkin@xxxxxxxxxxxx> > >>>> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > >>>> Cc: linux-arm-msm@xxxxxxxxxxxxxxx > >>>> Cc: freedreno@xxxxxxxxxxxxxxxxxxxxx > >>>> Cc: nouveau@xxxxxxxxxxxxxxxxxxxxx > >>>> Cc: linux-tegra@xxxxxxxxxxxxxxx > >>>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >>>> --- > >>>> 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_v6_0.c | 3 ++- > >>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- > >>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 ++-- > >>>> drivers/gpu/drm/bridge/sii902x.c | 3 ++- > >>>> drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- > >>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- > >>>> drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++--------- > >>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++- > >>>> drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++- > >>>> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++----- > >>>> drivers/gpu/drm/i915/intel_lspcon.c | 15 ++++++----- > >>>> drivers/gpu/drm/i915/intel_sdvo.c | 10 ++++--- > >>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 ++- > >>>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 3 ++- > >>>> drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++-- > >>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 5 ++-- > >>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +- > >>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++- > >>>> drivers/gpu/drm/sti/sti_hdmi.c | 3 ++- > >>>> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 3 ++- > >>>> drivers/gpu/drm/tegra/hdmi.c | 3 ++- > >>>> drivers/gpu/drm/tegra/sor.c | 3 ++- > >>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 11 +++++--- > >>>> drivers/gpu/drm/zte/zx_hdmi.c | 4 ++- > >>>> include/drm/drm_edid.h | 8 +++--- > >>>> 27 files changed, 94 insertions(+), 66 deletions(-) > >>> For dw-hdmi and omapdrm, > >>> > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > >>> -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
