| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <fa8e151f-a8c7-44fe-a8bf-949d5900112d@linaro.org>
Date: Mon, 19 Aug 2024 18:01:04 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Jerome Brunet <jbrunet@...libre.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>
Cc: Kevin Hilman <khilman@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
dri-devel@...ts.freedesktop.org, linux-amlogic@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/9] drm/meson: hdmi: move encoder settings out of phy
driver
On 30/07/2024 14:50, Jerome Brunet wrote:
> This relocates register pokes of the HDMI VPU encoder out of the
> HDMI phy driver. As far as HDMI is concerned, the sequence in which
> the setup is done remains mostly the same.
>
> This was tested with modetest, cycling through the following resolutions:
> #0 3840x2160 60.00
> #1 3840x2160 59.94
> #2 3840x2160 50.00
> #3 3840x2160 30.00
> #4 3840x2160 29.97
> #5 3840x2160 25.00
> #6 3840x2160 24.00
> #7 3840x2160 23.98
> #8 1920x1080 60.00
> #9 1920x1080 60.00
> #10 1920x1080 59.94
> #11 1920x1080i 30.00
> #12 1920x1080i 29.97
> #13 1920x1080 50.00
> #14 1920x1080i 25.00
> #15 1920x1080 30.00
> #16 1920x1080 29.97
> #17 1920x1080 25.00
> #18 1920x1080 24.00
> #19 1920x1080 23.98
> #20 1280x1024 60.02
> #21 1152x864 59.97
> #22 1280x720 60.00
> #23 1280x720 59.94
> #24 1280x720 50.00
> #25 1024x768 60.00
> #26 800x600 60.32
> #27 720x576 50.00
> #28 720x480 59.94
>
> No regression to report.
>
> This is part of an effort to clean up Amlogic HDMI related drivers which
> should eventually allow to stop using the component API and HHI syscon.
>
> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
> ---
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 38 ----------------------
> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 16 +++++++++
> 2 files changed, 16 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 5565f7777529..bcf4f83582f2 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -115,12 +115,6 @@
>
> static DEFINE_SPINLOCK(reg_lock);
>
> -enum meson_venc_source {
> - MESON_VENC_SOURCE_NONE = 0,
> - MESON_VENC_SOURCE_ENCI = 1,
> - MESON_VENC_SOURCE_ENCP = 2,
> -};
> -
> struct meson_dw_hdmi;
>
> struct meson_dw_hdmi_data {
> @@ -376,8 +370,6 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
> struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data;
> bool is_hdmi2_sink = display->hdmi.scdc.supported;
> struct meson_drm *priv = dw_hdmi->priv;
> - unsigned int wr_clk =
> - readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
> bool mode_is_420 = false;
>
> DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
> @@ -421,36 +413,6 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
> meson_dw_hdmi_phy_reset(dw_hdmi);
> meson_dw_hdmi_phy_reset(dw_hdmi);
>
> - /* Temporary Disable VENC video stream */
> - if (priv->venc.hdmi_use_enci)
> - writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
> - else
> - writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
> -
> - /* Temporary Disable HDMI video stream to HDMI-TX */
> - writel_bits_relaxed(0x3, 0,
> - priv->io_base + _REG(VPU_HDMI_SETTING));
> - writel_bits_relaxed(0xf << 8, 0,
> - priv->io_base + _REG(VPU_HDMI_SETTING));
> -
> - /* Re-Enable VENC video stream */
> - if (priv->venc.hdmi_use_enci)
> - writel_relaxed(1, priv->io_base + _REG(ENCI_VIDEO_EN));
> - else
> - writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
> -
> - /* Push back HDMI clock settings */
> - writel_bits_relaxed(0xf << 8, wr_clk & (0xf << 8),
> - priv->io_base + _REG(VPU_HDMI_SETTING));
> -
> - /* Enable and Select HDMI video source for HDMI-TX */
> - if (priv->venc.hdmi_use_enci)
> - writel_bits_relaxed(0x3, MESON_VENC_SOURCE_ENCI,
> - priv->io_base + _REG(VPU_HDMI_SETTING));
> - else
> - writel_bits_relaxed(0x3, MESON_VENC_SOURCE_ENCP,
> - priv->io_base + _REG(VPU_HDMI_SETTING));
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 0593a1cde906..1c3e3e5526eb 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -45,6 +45,12 @@ struct meson_encoder_hdmi {
> struct cec_notifier *cec_notifier;
> };
>
> +enum meson_venc_source {
> + MESON_VENC_SOURCE_NONE = 0,
> + MESON_VENC_SOURCE_ENCI = 1,
> + MESON_VENC_SOURCE_ENCP = 2,
> +};
> +
> #define bridge_to_meson_encoder_hdmi(x) \
> container_of(x, struct meson_encoder_hdmi, bridge)
>
> @@ -247,6 +253,14 @@ static void meson_encoder_hdmi_atomic_enable(struct drm_bridge *bridge,
> writel_relaxed(1, priv->io_base + _REG(ENCI_VIDEO_EN));
> else
> writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
> +
> + /* Enable and Select HDMI video source for HDMI-TX */
> + if (priv->venc.hdmi_use_enci)
> + writel_bits_relaxed(0x3, MESON_VENC_SOURCE_ENCI,
> + priv->io_base + _REG(VPU_HDMI_SETTING));
> + else
> + writel_bits_relaxed(0x3, MESON_VENC_SOURCE_ENCP,
> + priv->io_base + _REG(VPU_HDMI_SETTING));
> }
>
> static void meson_encoder_hdmi_atomic_disable(struct drm_bridge *bridge,
> @@ -257,6 +271,8 @@ static void meson_encoder_hdmi_atomic_disable(struct drm_bridge *bridge,
>
> writel_bits_relaxed(0x3, 0,
> priv->io_base + _REG(VPU_HDMI_SETTING));
> + writel_bits_relaxed(0xf << 8, 0,
> + priv->io_base + _REG(VPU_HDMI_SETTING));
>
> writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
> writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
Nice usage of the split bridge architecture!
Now we must make sure the atomic enable order doesn't change...
Reviewed-by: Neil Armstrong <neil.armstrong@...aro.org>
Powered by blists - more mailing lists