[Nouveau] [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Dec 5 06:32:45 UTC 2018
Hi Ville,
On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote:
> 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:
> >>>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >>>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>>>>
> >>>>> 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.
Or a bridge state ? With chained bridges the mode can vary along the pipeline,
the CRTC adjusted mode will only cover the link between the CRTC and the first
bridge. It's only a matter of time until we need to store other intermediate
modes in states. I'd rather prepare for that instead of passing the CRTC state
to bridges.
> > In moment of creating infoframe it should be set properly.
> >
> >>>> Please see below for an additional comment.
> >>>>
> >>>>> Cc: Alex Deucher <alexander.deucher at amd.com>
> >>>>> Cc: "Christian König" <christian.koenig at amd.com>
> >>>>> Cc: "David (ChunMing) Zhou" <David1.Zhou at amd.com>
> >>>>> Cc: Archit Taneja <architt at codeaurora.org>
> >>>>> Cc: Andrzej Hajda <a.hajda at samsung.com>
> >>>>> Cc: Laurent Pinchart <Laurent.pinchart at ideasonboard.com>
> >>>>> Cc: Inki Dae <inki.dae at samsung.com>
> >>>>> Cc: Joonyoung Shim <jy0922.shim at samsung.com>
> >>>> Cc: Seung-Woo Kim <sw0312.kim at samsung.com>
> >>>>> Cc: Kyungmin Park <kyungmin.park at samsung.com>
> >>>>> Cc: Russell King <linux at armlinux.org.uk>
> >>>>> Cc: CK Hu <ck.hu at mediatek.com>
> >>>>> Cc: Philipp Zabel <p.zabel at pengutronix.de>
> >>>>> Cc: Rob Clark <robdclark at gmail.com>
> >>>>> Cc: Ben Skeggs <bskeggs at redhat.com>
> >>>>> Cc: Tomi Valkeinen <tomi.valkeinen at ti.com>
> >>>>> Cc: Sandy Huang <hjc at rock-chips.com>
> >>>>> Cc: "Heiko Stübner" <heiko at sntech.de>
> >>>>> Cc: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> >>>>> Cc: Vincent Abriou <vincent.abriou at st.com>
> >>>>> Cc: Thierry Reding <thierry.reding at gmail.com>
> >>>>> Cc: Eric Anholt <eric at anholt.net>
> >>>>> Cc: Shawn Guo <shawnguo at kernel.org>
> >>>>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> >>>>> Cc: amd-gfx at lists.freedesktop.org
> >>>>> Cc: linux-arm-msm at vger.kernel.org
> >>>>> Cc: freedreno at lists.freedesktop.org
> >>>>> Cc: nouveau at lists.freedesktop.org
> >>>>> Cc: linux-tegra at vger.kernel.org
> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.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_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 at ideasonboard.com>
--
Regards,
Laurent Pinchart
More information about the Nouveau
mailing list