[PATCH v1 0/3] retrieve DSI DSC through DRM bridge

[PATCH v1 0/3] retrieve DSI DSC through DRM bridge

[Kuogee Hsieh]

Add msm_dsi_bridge_get_dsc_config() to retrieve DSI DSC through drm bridge. 
After that remove msm_dsi_get_dsc_config().

Kuogee Hsieh (3):
  drm/msm/dsi: add msm_dsi_bridge_get_dsc_config()
  drm/msm/dpu: retrieve DSI DSC struct at atomic_check()
  drm/msm/dpu: remove msm_dsi_get_dsc_config()

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 17 ++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  2 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  2 --
 drivers/gpu/drm/msm/dsi/dsi.c               |  5 ++++-
 drivers/gpu/drm/msm/dsi/dsi.h               |  2 ++
 drivers/gpu/drm/msm/dsi/dsi_manager.c       |  4 ++--
 drivers/gpu/drm/msm/msm_drv.h               |  4 ++--
 7 files changed, 24 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH v1 1/3] drm/msm/dsi: add msm_dsi_bridge_get_dsc_config()

[Kuogee Hsieh]

DSI is implemented as a DRM bridge. It is more logically to access
DSI from outside world through DRM bridge structure. Add DSI bridge
function msm_dsi_bridge_get_dsc_config() to retrieve DSC information.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi.c         | 8 ++++++++
 drivers/gpu/drm/msm/dsi/dsi.h         | 2 ++
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 ++--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index baab79a..fa7fcb5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -17,6 +17,14 @@ struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi)
 	return msm_dsi_host_get_dsc_config(msm_dsi->host);
 }
 
+struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge)
+{
+        int id = dsi_mgr_bridge_get_id(bridge);
+        struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
+
+	return msm_dsi_host_get_dsc_config(msm_dsi->host);
+}
+
 static int dsi_get_phy(struct msm_dsi *msm_dsi)
 {
 	struct platform_device *pdev = msm_dsi->pdev;
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index bd3763a..394f605 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -64,6 +64,8 @@ bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len);
 int msm_dsi_manager_register(struct msm_dsi *msm_dsi);
 void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi);
 void msm_dsi_manager_tpg_enable(void);
+int dsi_mgr_bridge_get_id(struct drm_bridge *bridge);
+struct msm_dsi *dsi_mgr_get_dsi(int id);
 
 /* msm dsi */
 static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 1bbac72..5a7ac338 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -60,7 +60,7 @@ static inline bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
 }
 #endif
 
-static inline struct msm_dsi *dsi_mgr_get_dsi(int id)
+struct msm_dsi *dsi_mgr_get_dsi(int id)
 {
 	return msm_dsim_glb.dsi[id];
 }
@@ -218,7 +218,7 @@ struct dsi_bridge {
 
 #define to_dsi_bridge(x) container_of(x, struct dsi_bridge, base)
 
-static int dsi_mgr_bridge_get_id(struct drm_bridge *bridge)
+int dsi_mgr_bridge_get_id(struct drm_bridge *bridge)
 {
 	struct dsi_bridge *dsi_bridge = to_dsi_bridge(bridge);
 	return dsi_bridge->id;
-- 
2.7.4

[PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Kuogee Hsieh]

At current implementation, DSI DSC struct is populated at display setup
during system bootup. This mechanism works fine with embedded display.
But will run into problem with plugin/unplug oriented external display,
such as DP, due to DSC struct will become stale once external display
unplugged. New DSC struct has to be re populated to reflect newer external
display which just plugged in. Move retrieving of DSI DSC struct to
atomic_check() so that same mechanism will work for both embedded display
and external plugin/unplug oriented display.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 ++++++++++++++-
 drivers/gpu/drm/msm/msm_drv.h               |  6 ++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3b416e1..2927d20 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -16,6 +16,8 @@
 #include <drm/drm_crtc.h>
 #include <drm/drm_file.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_fixed.h>
 
 #include "msm_drv.h"
 #include "dpu_kms.h"
@@ -639,6 +641,15 @@ static int dpu_encoder_virt_atomic_check(
 		}
 	}
 
+	if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
+		struct drm_bridge *bridge;
+
+		if (!dpu_enc->dsc) {
+			bridge = drm_bridge_chain_get_first_bridge(drm_enc);
+			dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);
+		}
+	}
+
 	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
 
 	/*
@@ -2121,8 +2132,10 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
 					phys_enc->hw_pp->merge_3d->idx);
 	}
 
-	if (dpu_enc->dsc)
+	if (dpu_enc->dsc) {
 		dpu_encoder_unprep_dsc(dpu_enc);
+		dpu_enc->dsc = NULL;
+	}
 
 	intf_cfg.stream_sel = 0; /* Don't care value for video mode */
 	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index e13a8cb..5a7c1f4 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -341,6 +341,7 @@ bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi);
 bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
 bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
 struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi);
+struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge);
 #else
 static inline void __init msm_dsi_register(void)
 {
@@ -374,6 +375,11 @@ static inline struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_
 {
 	return NULL;
 }
+
+struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge)
+{
+	return NULL;
+}
 #endif
 
 #ifdef CONFIG_DRM_MSM_DP
-- 
2.7.4

Re: [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Dmitry Baryshkov]

On 30/05/2023 19:31, Kuogee Hsieh wrote:
> At current implementation, DSI DSC struct is populated at display setup
> during system bootup. This mechanism works fine with embedded display.
> But will run into problem with plugin/unplug oriented external display,
> such as DP, due to DSC struct will become stale once external display
> unplugged. New DSC struct has to be re populated to reflect newer external
> display which just plugged in. Move retrieving of DSI DSC struct to
> atomic_check() so that same mechanism will work for both embedded display
> and external plugin/unplug oriented display.
> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 15 ++++++++++++++-
>   drivers/gpu/drm/msm/msm_drv.h               |  6 ++++++
>   2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 3b416e1..2927d20 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -16,6 +16,8 @@
>   #include <drm/drm_crtc.h>
>   #include <drm/drm_file.h>
>   #include <drm/drm_probe_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_fixed.h>
>   
>   #include "msm_drv.h"
>   #include "dpu_kms.h"
> @@ -639,6 +641,15 @@ static int dpu_encoder_virt_atomic_check(
>   		}
>   	}
>   
> +	if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {

INTF_DSI

> +		struct drm_bridge *bridge;
> +
> +		if (!dpu_enc->dsc) {

This condition is not correct. We should be updating the DSC even if 
there is one.

> +			bridge = drm_bridge_chain_get_first_bridge(drm_enc);
> +			dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);

This approach will not work for the hot-pluggable outputs. The dpu_enc 
is not a part of the state. It should not be touched before 
atomic_commit actually commits changes.

Also, I don't think I like the API. It makes it impossible for the 
driver to check that the bridge is the actually our DSI bridge or not.
Once you add DP here, the code will explode.

I think instead we should extend the drm_bridge API to be able to get 
the DSC configuration from it directly. Additional care should be put to 
design an assymetrical API. Theoretically a drm_bridge can be both DSC 
source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge, 
supporting DSC on the DSI side too.

> +		}
> +	}
> +
>   	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
>   
>   	/*
> @@ -2121,8 +2132,10 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
>   					phys_enc->hw_pp->merge_3d->idx);
>   	}
>   
> -	if (dpu_enc->dsc)
> +	if (dpu_enc->dsc) {
>   		dpu_encoder_unprep_dsc(dpu_enc);
> +		dpu_enc->dsc = NULL;
> +	}
>   
>   	intf_cfg.stream_sel = 0; /* Don't care value for video mode */
>   	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index e13a8cb..5a7c1f4 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -341,6 +341,7 @@ bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi);
>   bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
>   bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
>   struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi);
> +struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge);
>   #else
>   static inline void __init msm_dsi_register(void)
>   {
> @@ -374,6 +375,11 @@ static inline struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_
>   {
>   	return NULL;
>   }
> +
> +struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge)
> +{
> +	return NULL;
> +}

These two chunks belong to the previous patch.

>   #endif
>   
>   #ifdef CONFIG_DRM_MSM_DP

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Kuogee Hsieh]

>>   +    if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
>
> INTF_DSI
>
>> +        struct drm_bridge *bridge;
>> +
>> +        if (!dpu_enc->dsc) {
>
> This condition is not correct. We should be updating the DSC even if 
> there is one.
>
>> +            bridge = drm_bridge_chain_get_first_bridge(drm_enc);
>> +            dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);
>
> This approach will not work for the hot-pluggable outputs. The dpu_enc 
> is not a part of the state. It should not be touched before 
> atomic_commit actually commits changes.
where can drm_dsc_config be stored?
>
> Also, I don't think I like the API. It makes it impossible for the 
> driver to check that the bridge is the actually our DSI bridge or not.
> Once you add DP here, the code will explode.
>
> I think instead we should extend the drm_bridge API to be able to get 
> the DSC configuration from it directly. Additional care should be put 
> to design an assymetrical API. Theoretically a drm_bridge can be both 
> DSC source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge, 
> supporting DSC on the DSI side too.

Form my understanding, a bridge contains two interfaces.

Therefore I would think only one bridge for dsi-to-dp bridge? and this 
bridge should represent the bridge chip?

I am thinking adding an ops function, get_bridge_dsc() to struct 
drm_bridge_funcs to retrieve drm_dsc_config.

Do you have other suggestion?

>

Re: [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Dmitry Baryshkov]

On Wed, 31 May 2023 at 18:41, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
>
>
> >>   +    if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
> >
> > INTF_DSI
> >
> >> +        struct drm_bridge *bridge;
> >> +
> >> +        if (!dpu_enc->dsc) {
> >
> > This condition is not correct. We should be updating the DSC even if
> > there is one.
> >
> >> +            bridge = drm_bridge_chain_get_first_bridge(drm_enc);
> >> +            dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);
> >
> > This approach will not work for the hot-pluggable outputs. The dpu_enc
> > is not a part of the state. It should not be touched before
> > atomic_commit actually commits changes.
> where can drm_dsc_config be stored?

I'd say, get it during atomic_check (and don't store it anywhere).
Then get it during atomic_enable (and save in dpu_enc).

> >
> > Also, I don't think I like the API. It makes it impossible for the
> > driver to check that the bridge is the actually our DSI bridge or not.
> > Once you add DP here, the code will explode.
> >
> > I think instead we should extend the drm_bridge API to be able to get
> > the DSC configuration from it directly. Additional care should be put
> > to design an assymetrical API. Theoretically a drm_bridge can be both
> > DSC source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge,
> > supporting DSC on the DSI side too.
>
> Form my understanding, a bridge contains two interfaces.
>
> Therefore I would think only one bridge for dsi-to-dp bridge? and this
> bridge should represent the bridge chip?
>
> I am thinking adding an ops function, get_bridge_dsc() to struct
> drm_bridge_funcs to retrieve drm_dsc_config.

So, for this DSI-to-DP bridge will get_bridge_dsc() return DSC
configuration for  the DSI or for the DP side of the bridge?

>
> Do you have other suggestion?

Let me think about it for a few days.

-- 
With best wishes
Dmitry

Re: [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Dmitry Baryshkov]

On Wed, 31 May 2023 at 20:29, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
>
>
> On 5/31/2023 10:12 AM, Dmitry Baryshkov wrote:
> > On Wed, 31 May 2023 at 18:41, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> >>
> >>
> >>>>    +    if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI) {
> >>> INTF_DSI
> >>>
> >>>> +        struct drm_bridge *bridge;
> >>>> +
> >>>> +        if (!dpu_enc->dsc) {
> >>> This condition is not correct. We should be updating the DSC even if
> >>> there is one.
> >>>
> >>>> +            bridge = drm_bridge_chain_get_first_bridge(drm_enc);
> >>>> +            dpu_enc->dsc = msm_dsi_bridge_get_dsc_config(bridge);
> >>> This approach will not work for the hot-pluggable outputs. The dpu_enc
> >>> is not a part of the state. It should not be touched before
> >>> atomic_commit actually commits changes.
> >> where can drm_dsc_config be stored?
> > I'd say, get it during atomic_check (and don't store it anywhere).
> > Then get it during atomic_enable (and save in dpu_enc).
> got it.
> >
> >>> Also, I don't think I like the API. It makes it impossible for the
> >>> driver to check that the bridge is the actually our DSI bridge or not.
> >>> Once you add DP here, the code will explode.
> >>>
> >>> I think instead we should extend the drm_bridge API to be able to get
> >>> the DSC configuration from it directly. Additional care should be put
> >>> to design an assymetrical API. Theoretically a drm_bridge can be both
> >>> DSC source and DSC sink. Imagine a DSI-to-DP or DSI-to-HDMI bridge,
> >>> supporting DSC on the DSI side too.
> >> Form my understanding, a bridge contains two interfaces.
> >>
> >> Therefore I would think only one bridge for dsi-to-dp bridge? and this
> >> bridge should represent the bridge chip?
> >>
> >> I am thinking adding an ops function, get_bridge_dsc() to struct
> >> drm_bridge_funcs to retrieve drm_dsc_config.
> > So, for this DSI-to-DP bridge will get_bridge_dsc() return DSC
> > configuration for  the DSI or for the DP side of the bridge?
>
> I would think should be DP side. there is no reason to enable dsc on
> both DSI and DP fro a bridge chip.

Well, there can be. E.g. to lower the clock rates of the DSI link.

>
> drm_bridge_chain_get_first_bridge(drm_enc) should return the bridge of
> the controller.
>
> In DSI-to-DP bridge chip case, this controller will be the bridge chip
> who configured to perform protocol conversion between DSI and DP.
>
> If DSC enabled should be at DP size which connect to panel.

Ok, so it returns the DSC configuration of the bridge's source side.
Now let's consider a panel bridge for the DSC-enabled DSI panel.
Should get_bridge_dsc() return a DSC config in this case?

> >> Do you have other suggestion?
> > Let me think about it for a few days.
-- 
With best wishes
Dmitry

Re: [Freedreno] [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

[Dmitry Baryshkov]

Generic note: please use reply-to-all instead of any other options to
answer the email. You have dropped all recipients (except the
freedreno@) in the message
<d1a320c4-d851-ba75-ef7b-80dc369d1cfd@quicinc.com> (and it was left
unnoticed).


On Fri, 2 Jun 2023 at 20:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> >> There is one option which is keep current
> >>
> >> 1) keep struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi
> >> *msm_dsi) at dsi.c
> >>
> >> 2) use  struct msm_display_info *disp_info saved at dpu_enc to locate
> >> struct msm_dsi from priv->dsi[] list (see item #3)
> >>
> >> 3)  info.dsc = msm_dsi_get_dsc_config(priv->dsi[info.h_tile_instance[0]]);
> >>
> >> 4) ballistically, keep original code but move  info.dsc =
> >> msm_dsi_get_dsc_config(priv->dsi[i]); to other place sush as
> >> atomic_check() and atomic_enable().
> >>
> > 5) leave drm_dsc_config handling as is, update the dsc config from the
> > DP driver as suitable. If DSC is not supported, set
> > dsc->dsc_version_major = 0 and dsc->dsc_version_minor = 0 on the DP
> > side. In DPU driver verify that dsc->dsc_version_major/_minor != 0.
>
> I am confusing with item 5)
>
> Currently, msm_dsi_get_dsc_config() of dsi side return dsc pointer if
> dsc enabled and NULL if dsc not enabled.
>
> Should checking dsc == NULL is good enough to differentiate between dsc
> is supported and not supported?

This is called a "shared memory area". Instead of either providing a
dynamic data pointer, one can provide a pointer to the static area
which is filled by DP or DSI. If there is no DSC available, one flags
'data not valid' by setting major,minor to 0.

>
> Why need to set both dsc->dsc_version_major = 0 and
> dsc->dsc_version_minor = 0 for dsc is not supported?

6) Another option (which is more in style of what is done in the
vendor kernel, if I'm not mistaken):

Enhance struct drm_display_mode to contain a pointer to the DSC
config. Use this pointer to check whether DSC should be enabled for
the particular mode or not. The panels with the static DSC
configuration can use a static data pointer.


-- 
With best wishes
Dmitry

[PATCH v1 3/3] drm/msm/dpu: remove msm_dsi_get_dsc_config()

[Kuogee Hsieh]

Since msm_dsi_bridge_get_dsc_config() was added to retrieve DSI DSC
info through DRM bridge, msm_dsi_get_dsc_config() become redundant
and should be removed.

Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 2 --
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 2 --
 drivers/gpu/drm/msm/dsi/dsi.c               | 5 -----
 drivers/gpu/drm/msm/msm_drv.h               | 6 ------
 5 files changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2927d20..1974d61 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2335,8 +2335,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
 		dpu_enc->idle_pc_supported =
 				dpu_kms->catalog->caps->has_idle_pc;
 
-	dpu_enc->dsc = disp_info->dsc;
-
 	mutex_lock(&dpu_enc->enc_lock);
 	for (i = 0; i < disp_info->num_of_h_tiles && !ret; i++) {
 		/*
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 2c9ef8d..03a9ca0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -28,7 +28,6 @@
  * @is_cmd_mode		Boolean to indicate if the CMD mode is requested
  * @is_te_using_watchdog_timer:  Boolean to indicate watchdog TE is
  *				 used instead of panel TE in cmd mode panels
- * @dsc:		DSC configuration data for DSC-enabled displays
  */
 struct msm_display_info {
 	int intf_type;
@@ -36,7 +35,6 @@ struct msm_display_info {
 	uint32_t h_tile_instance[MAX_H_TILES_PER_DISPLAY];
 	bool is_cmd_mode;
 	bool is_te_using_watchdog_timer;
-	struct drm_dsc_config *dsc;
 };
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index c24f487..2390e5c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -554,8 +554,6 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
 		info.h_tile_instance[info.num_of_h_tiles++] = i;
 		info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
 
-		info.dsc = msm_dsi_get_dsc_config(priv->dsi[i]);
-
 		if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
 			rc = msm_dsi_modeset_init(priv->dsi[other], dev, encoder);
 			if (rc) {
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index fa7fcb5..154f19e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -12,11 +12,6 @@ bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
 	return !(host_flags & MIPI_DSI_MODE_VIDEO);
 }
 
-struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi)
-{
-	return msm_dsi_host_get_dsc_config(msm_dsi->host);
-}
-
 struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge)
 {
         int id = dsi_mgr_bridge_get_id(bridge);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 5a7c1f4..8792e98 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -340,7 +340,6 @@ void msm_dsi_snapshot(struct msm_disp_state *disp_state, struct msm_dsi *msm_dsi
 bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi);
 bool msm_dsi_is_bonded_dsi(struct msm_dsi *msm_dsi);
 bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi);
-struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi);
 struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge);
 #else
 static inline void __init msm_dsi_register(void)
@@ -371,11 +370,6 @@ static inline bool msm_dsi_is_master_dsi(struct msm_dsi *msm_dsi)
 	return false;
 }
 
-static inline struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi *msm_dsi)
-{
-	return NULL;
-}
-
 struct drm_dsc_config *msm_dsi_bridge_get_dsc_config(struct drm_bridge *bridge)
 {
 	return NULL;
-- 
2.7.4

Re: [PATCH v1 3/3] drm/msm/dpu: remove msm_dsi_get_dsc_config()

[Dmitry Baryshkov]

On 30/05/2023 19:31, Kuogee Hsieh wrote:
> Since msm_dsi_bridge_get_dsc_config() was added to retrieve DSI DSC
> info through DRM bridge, msm_dsi_get_dsc_config() become redundant
> and should be removed.
> 
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 --
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 2 --
>   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     | 2 --
>   drivers/gpu/drm/msm/dsi/dsi.c               | 5 -----
>   drivers/gpu/drm/msm/msm_drv.h               | 6 ------
>   5 files changed, 17 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry