| 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: <63e5ddf6-151a-42aa-b2cf-003d91b34a04@quicinc.com>
Date: Wed, 23 Apr 2025 19:04:16 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Rob Clark
<robdclark@...il.com>, Sean Paul <sean@...rly.run>,
Marijn Suijten
<marijn.suijten@...ainline.org>,
Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>,
Krzysztof
Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn
Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>
CC: <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 6/7] drm/msm/mdp4: switch LVDS to use
drm_bridge/_connector
On 2/26/2025 6:25 PM, Dmitry Baryshkov wrote:
> LVDS support in MDP4 driver makes use of drm_connector directly. However
> LCDC encoder and LVDS connector are wrappers around drm_panel. Switch
> them to use drm_panel_bridge/drm_bridge_connector. This allows using
> standard interface for the drm_panel and also inserting additional
> bridges between encoder and panel.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
> drivers/gpu/drm/msm/Makefile | 1 -
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 34 +++++--
> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 6 +-
> drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c | 20 +----
> .../gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c | 100 ---------------------
> 5 files changed, 28 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 5df20cbeafb8bf07c825a1fd72719d5a56c38613..7a2ada6e2d74a902879e4f12a78ed475e5209ec2 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -48,7 +48,6 @@ msm-display-$(CONFIG_DRM_MSM_MDP4) += \
> disp/mdp4/mdp4_dsi_encoder.o \
> disp/mdp4/mdp4_dtv_encoder.o \
> disp/mdp4/mdp4_lcdc_encoder.o \
> - disp/mdp4/mdp4_lvds_connector.o \
> disp/mdp4/mdp4_lvds_pll.o \
> disp/mdp4/mdp4_irq.o \
> disp/mdp4/mdp4_kms.o \
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 689e210660a5218ed1e2d116073723215af5a187..93c9411eb422bc67b7fedb5ffce4c330310b520f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -6,6 +6,8 @@
>
> #include <linux/delay.h>
>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_bridge_connector.h>
> #include <drm/drm_vblank.h>
>
> #include "msm_drv.h"
> @@ -189,7 +191,7 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms,
> struct msm_drm_private *priv = dev->dev_private;
> struct drm_encoder *encoder;
> struct drm_connector *connector;
> - struct device_node *panel_node;
> + struct drm_bridge *next_bridge;
> int dsi_id;
> int ret;
>
> @@ -199,27 +201,43 @@ static int mdp4_modeset_init_intf(struct mdp4_kms *mdp4_kms,
> * bail out early if there is no panel node (no need to
> * initialize LCDC encoder and LVDS connector)
> */
> - panel_node = of_graph_get_remote_node(dev->dev->of_node, 0, 0);
> - if (!panel_node)
> - return 0;
> + next_bridge = devm_drm_of_get_bridge(dev->dev, dev->dev->of_node, 0, 0);
> + if (IS_ERR(next_bridge)) {
> + ret = PTR_ERR(next_bridge);
> + if (ret == -ENODEV)
> + return 0;
> + return ret;
> + }
>
> - encoder = mdp4_lcdc_encoder_init(dev, panel_node);
> + encoder = mdp4_lcdc_encoder_init(dev);
> if (IS_ERR(encoder)) {
> DRM_DEV_ERROR(dev->dev, "failed to construct LCDC encoder\n");
> - of_node_put(panel_node);
> return PTR_ERR(encoder);
> }
>
> /* LCDC can be hooked to DMA_P (TODO: Add DMA_S later?) */
> encoder->possible_crtcs = 1 << DMA_P;
>
> - connector = mdp4_lvds_connector_init(dev, panel_node, encoder);
> + ret = drm_bridge_attach(encoder, next_bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> + if (ret) {
> + DRM_DEV_ERROR(dev->dev, "failed to attach LVDS panel/bridge: %d\n", ret);
> +
> + return ret;
> + }
Can you pls point me to the lvds bridge used with this apq8064 board? I
was unable to find it. Just wanted to compare that against this while
reviewing.
> +
> + connector = drm_bridge_connector_init(dev, encoder);
> if (IS_ERR(connector)) {
> DRM_DEV_ERROR(dev->dev, "failed to initialize LVDS connector\n");
> - of_node_put(panel_node);
> return PTR_ERR(connector);
> }
>
> + ret = drm_connector_attach_encoder(connector, encoder);
> + if (ret) {
> + DRM_DEV_ERROR(dev->dev, "failed to attach LVDS connector: %d\n", ret);
> +
> + return ret;
> + }
> +
> break;
> case DRM_MODE_ENCODER_TMDS:
> encoder = mdp4_dtv_encoder_init(dev);
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> index e0380d3b7e0cee99c4c376bf6369887106f44ede..306f5ca8f810aaeecea56e74065933bbffcb67ec 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h
> @@ -191,11 +191,7 @@ struct drm_crtc *mdp4_crtc_init(struct drm_device *dev,
> long mdp4_dtv_round_pixclk(struct drm_encoder *encoder, unsigned long rate);
> struct drm_encoder *mdp4_dtv_encoder_init(struct drm_device *dev);
>
> -struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev,
> - struct device_node *panel_node);
> -
> -struct drm_connector *mdp4_lvds_connector_init(struct drm_device *dev,
> - struct device_node *panel_node, struct drm_encoder *encoder);
> +struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev);
>
> #ifdef CONFIG_DRM_MSM_DSI
> struct drm_encoder *mdp4_dsi_encoder_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> index cfcedd8a635cf0297365e845ef415a8f0d553183..a4f3edabefbd06286bfb8fbcd7f8c0a4281e5ef1 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lcdc_encoder.c
> @@ -14,7 +14,6 @@
>
> struct mdp4_lcdc_encoder {
> struct drm_encoder base;
> - struct device_node *panel_node;
> struct drm_panel *panel;
> struct clk *lcdc_clk;
> unsigned long int pixclock;
> @@ -262,19 +261,12 @@ static void mdp4_lcdc_encoder_disable(struct drm_encoder *encoder)
> struct mdp4_lcdc_encoder *mdp4_lcdc_encoder =
> to_mdp4_lcdc_encoder(encoder);
> struct mdp4_kms *mdp4_kms = get_kms(encoder);
> - struct drm_panel *panel;
>
> if (WARN_ON(!mdp4_lcdc_encoder->enabled))
> return;
>
> mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 0);
>
> - panel = of_drm_find_panel(mdp4_lcdc_encoder->panel_node);
> - if (!IS_ERR(panel)) {
> - drm_panel_disable(panel);
> - drm_panel_unprepare(panel);
> - }
> -
> /*
> * Wait for a vsync so we know the ENABLE=0 latched before
> * the (connector) source of the vsync's gets disabled,
> @@ -300,7 +292,6 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder *encoder)
> to_mdp4_lcdc_encoder(encoder);
> unsigned long pc = mdp4_lcdc_encoder->pixclock;
> struct mdp4_kms *mdp4_kms = get_kms(encoder);
> - struct drm_panel *panel;
> uint32_t config;
> int ret;
>
> @@ -335,12 +326,6 @@ static void mdp4_lcdc_encoder_enable(struct drm_encoder *encoder)
> if (ret)
> DRM_DEV_ERROR(dev->dev, "failed to enable lcdc_clk: %d\n", ret);
>
> - panel = of_drm_find_panel(mdp4_lcdc_encoder->panel_node);
> - if (!IS_ERR(panel)) {
> - drm_panel_prepare(panel);
> - drm_panel_enable(panel);
> - }
> -
> setup_phy(encoder);
>
> mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 1);
> @@ -375,8 +360,7 @@ static const struct drm_encoder_helper_funcs mdp4_lcdc_encoder_helper_funcs = {
> };
>
> /* initialize encoder */
> -struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev,
> - struct device_node *panel_node)
> +struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev)
> {
> struct drm_encoder *encoder;
> struct mdp4_lcdc_encoder *mdp4_lcdc_encoder;
> @@ -387,8 +371,6 @@ struct drm_encoder *mdp4_lcdc_encoder_init(struct drm_device *dev,
> if (IS_ERR(mdp4_lcdc_encoder))
> return ERR_CAST(mdp4_lcdc_encoder);
>
> - mdp4_lcdc_encoder->panel_node = panel_node;
> -
> encoder = &mdp4_lcdc_encoder->base;
>
> drm_encoder_helper_add(encoder, &mdp4_lcdc_encoder_helper_funcs);
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
> deleted file mode 100644
> index 4755eb13ef79f313d2be088145c8cd2e615226fe..0000000000000000000000000000000000000000
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_connector.c
> +++ /dev/null
> @@ -1,100 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (C) 2014 Red Hat
> - * Author: Rob Clark <robdclark@...il.com>
> - * Author: Vinay Simha <vinaysimha@...orcecomputing.com>
> - */
> -
> -#include "mdp4_kms.h"
> -
> -struct mdp4_lvds_connector {
> - struct drm_connector base;
> - struct drm_encoder *encoder;
> - struct device_node *panel_node;
> - struct drm_panel *panel;
> -};
> -#define to_mdp4_lvds_connector(x) container_of(x, struct mdp4_lvds_connector, base)
> -
> -static enum drm_connector_status mdp4_lvds_connector_detect(
> - struct drm_connector *connector, bool force)
> -{
> - struct mdp4_lvds_connector *mdp4_lvds_connector =
> - to_mdp4_lvds_connector(connector);
> -
> - if (!mdp4_lvds_connector->panel) {
> - mdp4_lvds_connector->panel =
> - of_drm_find_panel(mdp4_lvds_connector->panel_node);
> - if (IS_ERR(mdp4_lvds_connector->panel))
> - mdp4_lvds_connector->panel = NULL;
> - }
> -
> - return mdp4_lvds_connector->panel ?
> - connector_status_connected :
> - connector_status_disconnected;
> -}
> -
> -static void mdp4_lvds_connector_destroy(struct drm_connector *connector)
> -{
> - struct mdp4_lvds_connector *mdp4_lvds_connector =
> - to_mdp4_lvds_connector(connector);
> -
> - drm_connector_cleanup(connector);
> -
> - kfree(mdp4_lvds_connector);
> -}
> -
> -static int mdp4_lvds_connector_get_modes(struct drm_connector *connector)
> -{
> - struct mdp4_lvds_connector *mdp4_lvds_connector =
> - to_mdp4_lvds_connector(connector);
> - struct drm_panel *panel = mdp4_lvds_connector->panel;
> - int ret = 0;
> -
> - if (panel)
> - ret = drm_panel_get_modes(panel, connector);
> -
> - return ret;
> -}
> -
> -static const struct drm_connector_funcs mdp4_lvds_connector_funcs = {
> - .detect = mdp4_lvds_connector_detect,
> - .fill_modes = drm_helper_probe_single_connector_modes,
> - .destroy = mdp4_lvds_connector_destroy,
> - .reset = drm_atomic_helper_connector_reset,
> - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
> -static const struct drm_connector_helper_funcs mdp4_lvds_connector_helper_funcs = {
> - .get_modes = mdp4_lvds_connector_get_modes,
> -};
> -
> -/* initialize connector */
> -struct drm_connector *mdp4_lvds_connector_init(struct drm_device *dev,
> - struct device_node *panel_node, struct drm_encoder *encoder)
> -{
> - struct drm_connector *connector = NULL;
> - struct mdp4_lvds_connector *mdp4_lvds_connector;
> -
> - mdp4_lvds_connector = kzalloc(sizeof(*mdp4_lvds_connector), GFP_KERNEL);
> - if (!mdp4_lvds_connector)
> - return ERR_PTR(-ENOMEM);
> -
> - mdp4_lvds_connector->encoder = encoder;
> - mdp4_lvds_connector->panel_node = panel_node;
> -
> - connector = &mdp4_lvds_connector->base;
> -
> - drm_connector_init(dev, connector, &mdp4_lvds_connector_funcs,
> - DRM_MODE_CONNECTOR_LVDS);
> - drm_connector_helper_add(connector, &mdp4_lvds_connector_helper_funcs);
> -
> - connector->polled = 0;
> -
> - connector->interlace_allowed = 0;
> - connector->doublescan_allowed = 0;
> -
> - drm_connector_attach_encoder(connector, encoder);
> -
> - return connector;
> -}
>
Powered by blists - more mailing lists