[PATCH 0/3] drm: move dsc data pointer from drm_panel to mipi_dsi_device

[PATCH 0/3] drm: move dsc data pointer from drm_panel to mipi_dsi_device

[Dmitry Baryshkov]

To properly support DSC the sink driver (panel) has to pass DSC pps data
to the source (DSI host). The commit 0f40ba48de3b ("drm/msm/dsi: Pass
DSC params to drm_panel") added a pointer to the DSC data to the struct
drm_panel. However this is not the ideal solution.

First, this leaves DSC-supporting DSI sink bridges (like ANX7625 which
support DSC decoding on the MIPI DSI inputs).

Second, this does not play well with the panel_bridge. Drivers depending
solely on the bridge chains will still have to lookup panel and fetch
data from it.

Last, but not least, the DSC data is not relevant for the wide variety
of panels including DPI and LVDS panels.

To solve all these problems, move struct drm_dsc_config pointer from
struct drm_panel to struct mipi_host_device. This way MIPI DSI host
driver receives DSC data during attach callback without additional
lookups.

Dependencies: this depends on the MSM DRM DSC patchset [1] being pulled
in through the MSM DRM tree.

[1] https://patchwork.freedesktop.org/series/102262/

Dmitry Baryshkov (3):
  drm/mipi-dsi: pass DSC data through the struct mipi_dsi_device
  drm/msm/dsi: fetch DSC pps payload from struct mipi_dsi_device
  drm/panel: drop DSC pps pointer

 drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++++++++++--------------
 include/drm/drm_mipi_dsi.h         |  2 ++
 include/drm/drm_panel.h            |  7 -------
 3 files changed, 13 insertions(+), 21 deletions(-)

-- 
2.35.1

[PATCH 1/3] drm/mipi-dsi: pass DSC data through the struct mipi_dsi_device

[Dmitry Baryshkov]

The commit 0f40ba48de3b ("drm/msm/dsi: Pass DSC params to drm_panel")
added a pointer to the DSC data to the struct drm_panel. However DSC
support is not limited to the DSI panels. MIPI DSI bridges can also
consume DSC command streams. Thus add struct drm_dsc_config pointer to
the struct mipi_dsi_device.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 include/drm/drm_mipi_dsi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 147e51b6d241..8b1c9be9b2a7 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -177,6 +177,7 @@ struct mipi_dsi_device_info {
  * @lp_rate: maximum lane frequency for low power mode in hertz, this should
  * be set to the real limits of the hardware, zero is only accepted for
  * legacy drivers
+ * @dsc: panel/bridge DSC pps payload to be sent
  */
 struct mipi_dsi_device {
 	struct mipi_dsi_host *host;
@@ -189,6 +190,7 @@ struct mipi_dsi_device {
 	unsigned long mode_flags;
 	unsigned long hs_rate;
 	unsigned long lp_rate;
+	struct drm_dsc_config *dsc;
 };
 
 #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
-- 
2.35.1

Re: [PATCH 1/3] drm/mipi-dsi: pass DSC data through the struct mipi_dsi_device

[Dmitry Baryshkov]

On 01/05/2022 18:12, Dmitry Baryshkov wrote:
> The commit 0f40ba48de3b ("drm/msm/dsi: Pass DSC params to drm_panel")
> added a pointer to the DSC data to the struct drm_panel. However DSC
> support is not limited to the DSI panels. MIPI DSI bridges can also
> consume DSC command streams. Thus add struct drm_dsc_config pointer to
> the struct mipi_dsi_device.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Gracious ping for the review from the drm core

> ---
>   include/drm/drm_mipi_dsi.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 147e51b6d241..8b1c9be9b2a7 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -177,6 +177,7 @@ struct mipi_dsi_device_info {
>    * @lp_rate: maximum lane frequency for low power mode in hertz, this should
>    * be set to the real limits of the hardware, zero is only accepted for
>    * legacy drivers
> + * @dsc: panel/bridge DSC pps payload to be sent
>    */
>   struct mipi_dsi_device {
>   	struct mipi_dsi_host *host;
> @@ -189,6 +190,7 @@ struct mipi_dsi_device {
>   	unsigned long mode_flags;
>   	unsigned long hs_rate;
>   	unsigned long lp_rate;
> +	struct drm_dsc_config *dsc;
>   };
>   
>   #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"


-- 
With best wishes
Dmitry

[PATCH 2/3] drm/msm/dsi: fetch DSC pps payload from struct mipi_dsi_device

[Dmitry Baryshkov]

Now that struct mipi_dsi_device provides DSC data, fetch it from the
mentioned struct rather than from the struct drm_panel itself. This
would allow supporting MIPI DSI bridges handling DSC on their input
side.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a95d5df52653..173df1fd3692 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1690,6 +1690,17 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
 	msm_host->lanes = dsi->lanes;
 	msm_host->format = dsi->format;
 	msm_host->mode_flags = dsi->mode_flags;
+	if (dsi->dsc) {
+		struct msm_display_dsc_config *dsc = msm_host->dsc;
+
+		if (!dsc) {
+			dsc = devm_kzalloc(&msm_host->pdev->dev, sizeof(*dsc), GFP_KERNEL);
+			if (!dsc)
+				return -ENOMEM;
+			dsc->drm = dsi->dsc;
+			msm_host->dsc = dsc;
+		}
+	}
 
 	/* Some gpios defined in panel DT need to be controlled by host */
 	ret = dsi_host_init_panel_gpios(msm_host, &dsi->dev);
@@ -2164,23 +2175,9 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
 {
 	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
 	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
-	struct drm_panel *panel;
 	int ret;
 
 	msm_host->dev = dev;
-	panel = msm_dsi_host_get_panel(&msm_host->base);
-
-	if (!IS_ERR(panel) && panel->dsc) {
-		struct msm_display_dsc_config *dsc = msm_host->dsc;
-
-		if (!dsc) {
-			dsc = devm_kzalloc(&msm_host->pdev->dev, sizeof(*dsc), GFP_KERNEL);
-			if (!dsc)
-				return -ENOMEM;
-			dsc->drm = panel->dsc;
-			msm_host->dsc = dsc;
-		}
-	}
 
 	ret = cfg_hnd->ops->tx_buf_alloc(msm_host, SZ_4K);
 	if (ret) {
-- 
2.35.1

[PATCH 3/3] drm/panel: drop DSC pps pointer

[Dmitry Baryshkov]

Complete the move of DSC data pointer from struct drm_panel to struct
mipi_dsi_device.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 include/drm/drm_panel.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index d279ee455f01..1ba2d424a53f 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -179,13 +179,6 @@ struct drm_panel {
 	 * Panel entry in registry.
 	 */
 	struct list_head list;
-
-	/**
-	 * @dsc:
-	 *
-	 * Panel DSC pps payload to be sent
-	 */
-	struct drm_dsc_config *dsc;
 };
 
 void drm_panel_init(struct drm_panel *panel, struct device *dev,
-- 
2.35.1

Re: [PATCH 0/3] drm: move dsc data pointer from drm_panel to mipi_dsi_device

[Vinod Koul]

On 01-05-22, 18:12, Dmitry Baryshkov wrote:
> To properly support DSC the sink driver (panel) has to pass DSC pps data
> to the source (DSI host). The commit 0f40ba48de3b ("drm/msm/dsi: Pass
> DSC params to drm_panel") added a pointer to the DSC data to the struct
> drm_panel. However this is not the ideal solution.
> 
> First, this leaves DSC-supporting DSI sink bridges (like ANX7625 which
> support DSC decoding on the MIPI DSI inputs).
> 
> Second, this does not play well with the panel_bridge. Drivers depending
> solely on the bridge chains will still have to lookup panel and fetch
> data from it.
> 
> Last, but not least, the DSC data is not relevant for the wide variety
> of panels including DPI and LVDS panels.
> 
> To solve all these problems, move struct drm_dsc_config pointer from
> struct drm_panel to struct mipi_host_device. This way MIPI DSI host
> driver receives DSC data during attach callback without additional
> lookups.

Reviewed-by: Vinod Koul <vkoul@kernel.org>

I tested this on my pixel3 and had to change how panel driver handles
this, with that it worked just fine

Tested-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod