[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m7brftsrxdikfeumbjkubeeleezka7mwjbchxefqgs4ybtca5n@ge3ay2olagq2>
Date: Thu, 20 Feb 2025 18:06:01 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Jun Nie <jun.nie@...aro.org>
Cc: Rob Clark <robdclark@...il.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>, Jessica Zhang <quic_jesszhan@...cinc.com>,
Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Krishna Manikandan <quic_mkrishn@...cinc.com>, linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 5/5] drm/msm/dsi: Support DSC for dual panel case
On Thu, Feb 20, 2025 at 11:42:28PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@...aro.org> 于2025年2月20日周四 18:39写道:
> >
> > On Thu, Feb 20, 2025 at 06:07:56PM +0800, Jun Nie wrote:
> > > There is dual DSI case that every DSI link is connected to an independent
> > > panel. In this dual panel case, the frame width for DSC on each link should
> > > be halved to support the usage case.
> >
> > Isn't it the case for the DSI panel utilizing two DSI links?
>
> The added case here is 2 DSI panel utilizing two DSI links, 1 DSI link
> in each panel.
> I assume default case is 1 panel with 2 DSI link, such as Marijn's case.
So it should be halved in your case, but not in Marijn's case? I can
suspect that if you are describing two DSI panels as a single instance,
you should also adjust drm_dsc_config accordingly (on the panel's side?)
Maybe drm_dsc_config.pic_width and drm_dsc_config.pic_height should be
set on the panel's side? But then, how will that function for the DSI
panels or bridges which can change the mode?
> > > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > > ---
> > > drivers/gpu/drm/msm/dsi/dsi.h | 3 ++-
> > > drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++----
> > > drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 ++++++++--
> > > 3 files changed, 19 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> > > index 35b90c462f637111159b204269ce908614a21586..5a8978bed9f4ca897b418ced60194042d9dd8d05 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi.h
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> > > @@ -74,7 +74,8 @@ void msm_dsi_host_enable_irq(struct mipi_dsi_host *host);
> > > void msm_dsi_host_disable_irq(struct mipi_dsi_host *host);
> > > int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > > struct msm_dsi_phy_shared_timings *phy_shared_timings,
> > > - bool is_bonded_dsi, struct msm_dsi_phy *phy);
> > > + bool is_bonded_dsi, bool is_dual_panel,
> > > + struct msm_dsi_phy *phy);
> > > int msm_dsi_host_power_off(struct mipi_dsi_host *host);
> > > int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
> > > const struct drm_display_mode *mode);
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > index 976c5d82a2efa0fc51657b8534675890be7c33a6..752a97f7181c30dade0a7745492bf16649b3197b 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -902,7 +902,8 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod
> > > }
> > > }
> > >
> > > -static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> > > +static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi,
> > > + bool is_dual_panel)
> > > {
> > > struct drm_display_mode *mode = msm_host->mode;
> > > u32 hs_start = 0, vs_start = 0; /* take sync start as 0 */
> > > @@ -947,7 +948,10 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
> > > return;
> > > }
> > >
> > > - dsc->pic_width = mode->hdisplay;
> > > + if (is_dual_panel)
> > > + dsc->pic_width = hdisplay;
> > > + else
> > > + dsc->pic_width = mode->hdisplay;
> > > dsc->pic_height = mode->vdisplay;
> > > DBG("Mode %dx%d\n", dsc->pic_width, dsc->pic_height);
> > >
> > > @@ -2369,7 +2373,8 @@ static void msm_dsi_sfpb_config(struct msm_dsi_host *msm_host, bool enable)
> > >
> > > int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > > struct msm_dsi_phy_shared_timings *phy_shared_timings,
> > > - bool is_bonded_dsi, struct msm_dsi_phy *phy)
> > > + bool is_bonded_dsi, bool is_dual_panel,
> > > + struct msm_dsi_phy *phy)
> > > {
> > > struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
> > > const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
> > > @@ -2412,7 +2417,7 @@ int msm_dsi_host_power_on(struct mipi_dsi_host *host,
> > > goto fail_disable_clk;
> > > }
> > >
> > > - dsi_timing_setup(msm_host, is_bonded_dsi);
> > > + dsi_timing_setup(msm_host, is_bonded_dsi, is_dual_panel);
> > > dsi_sw_reset(msm_host);
> > > dsi_ctrl_enable(msm_host, phy_shared_timings, phy);
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > index be13bf682a9601484c9c14e8419563f37c2281ee..158b6cc907cb39cc3b182d3088b793d322a3527c 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > > @@ -24,6 +24,7 @@ struct msm_dsi_manager {
> > > struct msm_dsi *dsi[DSI_MAX];
> > >
> > > bool is_bonded_dsi;
> > > + bool is_dual_panel;
> > > bool is_sync_needed;
> > > int master_dsi_link_id;
> > > };
> > > @@ -31,6 +32,7 @@ struct msm_dsi_manager {
> > > static struct msm_dsi_manager msm_dsim_glb;
> > >
> > > #define IS_BONDED_DSI() (msm_dsim_glb.is_bonded_dsi)
> > > +#define IS_DUAL_PANEL() (msm_dsim_glb.is_dual_panel)
> > > #define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
> > > #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
> > >
> > > @@ -55,6 +57,7 @@ static int dsi_mgr_parse_of(struct device_node *np, int id)
> > > msm_dsim->is_bonded_dsi = of_property_read_bool(np, "qcom,dual-dsi-mode");
> > >
> > > if (msm_dsim->is_bonded_dsi) {
> > > + msm_dsim->is_dual_panel = of_property_read_bool(np, "qcom,dual-panel");
> > > if (of_property_read_bool(np, "qcom,master-dsi"))
> > > msm_dsim->master_dsi_link_id = id;
> > > if (!msm_dsim->is_sync_needed)
> > > @@ -214,6 +217,7 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > > struct mipi_dsi_host *host = msm_dsi->host;
> > > struct msm_dsi_phy_shared_timings phy_shared_timings[DSI_MAX];
> > > bool is_bonded_dsi = IS_BONDED_DSI();
> > > + bool is_dual_panel = IS_DUAL_PANEL();
> > > int ret;
> > >
> > > DBG("id=%d", id);
> > > @@ -222,7 +226,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > > if (ret)
> > > goto phy_en_fail;
> > >
> > > - ret = msm_dsi_host_power_on(host, &phy_shared_timings[id], is_bonded_dsi, msm_dsi->phy);
> > > + ret = msm_dsi_host_power_on(host, &phy_shared_timings[id],
> > > + is_bonded_dsi, is_dual_panel, msm_dsi->phy);
> > > if (ret) {
> > > pr_err("%s: power on host %d failed, %d\n", __func__, id, ret);
> > > goto host_on_fail;
> > > @@ -230,7 +235,8 @@ static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> > >
> > > if (is_bonded_dsi && msm_dsi1) {
> > > ret = msm_dsi_host_power_on(msm_dsi1->host,
> > > - &phy_shared_timings[DSI_1], is_bonded_dsi, msm_dsi1->phy);
> > > + &phy_shared_timings[DSI_1], is_bonded_dsi,
> > > + is_dual_panel, msm_dsi1->phy);
> > > if (ret) {
> > > pr_err("%s: power on host1 failed, %d\n",
> > > __func__, ret);
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
Powered by blists - more mailing lists