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.syrj...@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?


>
>> 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

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.deuc...@amd.com>
>>>> Cc: "Christian König" <christian.koe...@amd.com>
>>>> Cc: "David (ChunMing) Zhou" <david1.z...@amd.com>
>>>> Cc: Archit Taneja <arch...@codeaurora.org>
>>>> Cc: Andrzej Hajda <a.ha...@samsung.com>
>>>> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>>>> Cc: Inki Dae <inki....@samsung.com>
>>>> Cc: Joonyoung Shim <jy0922.s...@samsung.com>
>>>> Cc: Seung-Woo Kim <sw0312....@samsung.com>
>>>> Cc: Kyungmin Park <kyungmin.p...@samsung.com>
>>>> Cc: Russell King <li...@armlinux.org.uk>
>>>> Cc: CK Hu <ck...@mediatek.com>
>>>> Cc: Philipp Zabel <p.za...@pengutronix.de>
>>>> Cc: Rob Clark <robdcl...@gmail.com>
>>>> Cc: Ben Skeggs <bske...@redhat.com>
>>>> Cc: Tomi Valkeinen <tomi.valkei...@ti.com>
>>>> Cc: Sandy Huang <h...@rock-chips.com>
>>>> Cc: "Heiko Stübner" <he...@sntech.de>
>>>> Cc: Benjamin Gaignard <benjamin.gaign...@linaro.org>
>>>> Cc: Vincent Abriou <vincent.abr...@st.com>
>>>> Cc: Thierry Reding <thierry.red...@gmail.com>
>>>> Cc: Eric Anholt <e...@anholt.net>
>>>> Cc: Shawn Guo <shawn...@kernel.org>
>>>> Cc: Ilia Mirkin <imir...@alum.mit.edu>
>>>> Cc: amd-...@lists.freedesktop.org
>>>> Cc: linux-arm-...@vger.kernel.org
>>>> Cc: freedr...@lists.freedesktop.org
>>>> Cc: nouv...@lists.freedesktop.org
>>>> Cc: linux-te...@vger.kernel.org
>>>> Signed-off-by: Ville Syrjälä <ville.syrj...@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.pinch...@ideasonboard.com>
>>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to