[PATCH 0/3] Fixes for the komeda driver

[PATCH 0/3] Fixes for the komeda driver

[Faiz Abbas]

The following patches add fixes to the komeda DPU driver.

Patch 1 fixes an issue where the crtc always expects both pipelines to
always have remote nodes populated.

Patch 2 is a cosmetic fix that ensures komeda does not print verbose
pipeline information unless the entire pipeline is actually up.

Patch 3 adds a 40 bit DMA mask for each pipeline.

Amjad Ouled-Ameur (1):
  drm/arm/komeda: update DMA mask to 40 bits

Faiz Abbas (2):
  drm/arm/komeda: Fix komeda probe failing if there are no links in the
    secondary pipeline
  drm/arm/komeda: Move pipeline prints to after the entire pipeline has
    been enabled

 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 45 ++++++++++++++-----
 .../gpu/drm/arm/display/komeda/komeda_drv.c   |  4 ++
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |  1 +
 .../drm/arm/display/komeda/komeda_pipeline.c  |  4 +-
 4 files changed, 41 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH 1/3] drm/arm/komeda: Fix komeda probe failing if there are no links in the secondary pipeline

[Faiz Abbas]

Since commit f7936d6beda9 ("drm/arm/komeda: Remove component framework and
add a simple encoder"), the devm_drm_of_get_bridge() call happens
regardless of whether any remote nodes are available on the pipeline. Fix
this by moving the bridge attach to its own function and calling it
conditional on there being an output link.

Fixes: f7936d6beda9 ("drm/arm/komeda: Remove component framework and add a simple encoder")
Signed-off-by: Faiz Abbas <faiz.abbas@arm.com>
---
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 43 ++++++++++++++-----
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 2c661f28410e..b645c5998230 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -5,6 +5,7 @@
  *
  */
 #include <linux/clk.h>
+#include <linux/of.h>
 #include <linux/pm_runtime.h>
 #include <linux/spinlock.h>
 
@@ -610,12 +611,34 @@ get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
 	return NULL;
 }
 
+static int komeda_attach_bridge(struct device *dev,
+				struct komeda_pipeline *pipe,
+				struct drm_encoder *encoder)
+{
+	struct drm_bridge *bridge;
+	int err;
+
+	bridge = devm_drm_of_get_bridge(dev, pipe->of_node,
+					KOMEDA_OF_PORT_OUTPUT, 0);
+	if (IS_ERR(bridge))
+		return dev_err_probe(dev, PTR_ERR(bridge), "remote bridge not found for pipe: %s\n",
+				     of_node_full_name(pipe->of_node));
+
+	err = drm_bridge_attach(encoder, bridge, NULL, 0);
+	if (err)
+		dev_err(dev, "bridge_attach() failed for pipe: %s\n",
+			of_node_full_name(pipe->of_node));
+
+	return err;
+}
+
 static int komeda_crtc_add(struct komeda_kms_dev *kms,
 			   struct komeda_crtc *kcrtc)
 {
 	struct drm_crtc *crtc = &kcrtc->base;
 	struct drm_device *base = &kms->base;
-	struct drm_bridge *bridge;
+	struct komeda_pipeline *pipe = kcrtc->master;
+	struct drm_encoder *encoder = &kcrtc->encoder;
 	int err;
 
 	err = drm_crtc_init_with_planes(base, crtc,
@@ -626,27 +649,25 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
 
 	drm_crtc_helper_add(crtc, &komeda_crtc_helper_funcs);
 
-	crtc->port = kcrtc->master->of_output_port;
+	crtc->port = pipe->of_output_port;
 
 	/* Construct an encoder for each pipeline and attach it to the remote
 	 * bridge
 	 */
 	kcrtc->encoder.possible_crtcs = drm_crtc_mask(crtc);
-	err = drm_simple_encoder_init(base, &kcrtc->encoder,
-				      DRM_MODE_ENCODER_TMDS);
+	err = drm_simple_encoder_init(base, encoder, DRM_MODE_ENCODER_TMDS);
 	if (err)
 		return err;
 
-	bridge = devm_drm_of_get_bridge(base->dev, kcrtc->master->of_node,
-					KOMEDA_OF_PORT_OUTPUT, 0);
-	if (IS_ERR(bridge))
-		return PTR_ERR(bridge);
-
-	err = drm_bridge_attach(&kcrtc->encoder, bridge, NULL, 0);
+	if (pipe->of_output_links[0]) {
+		err = komeda_attach_bridge(base->dev, pipe, encoder);
+		if (err)
+			return err;
+	}
 
 	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
 
-	return err;
+	return 0;
 }
 
 int komeda_kms_add_crtcs(struct komeda_kms_dev *kms, struct komeda_dev *mdev)
-- 
2.25.1

Re: [PATCH 1/3] drm/arm/komeda: Fix komeda probe failing if there are no links in the secondary pipeline

[Liviu Dudau]

Hi Faiz,

On Mon, Feb 19, 2024 at 03:39:13PM +0530, Faiz Abbas wrote:
> Since commit f7936d6beda9 ("drm/arm/komeda: Remove component framework and
> add a simple encoder"), the devm_drm_of_get_bridge() call happens
> regardless of whether any remote nodes are available on the pipeline. Fix
> this by moving the bridge attach to its own function and calling it
> conditional on there being an output link.
> 
> Fixes: f7936d6beda9 ("drm/arm/komeda: Remove component framework and add a simple encoder")

I don't know what tree you're using, but the commit that you're quoting here is
actually 4cfe5cc02e3f. I will fix it when I merge the patch.

Best regards,
Liviu

> Signed-off-by: Faiz Abbas <faiz.abbas@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 43 ++++++++++++++-----
>  1 file changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 2c661f28410e..b645c5998230 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -5,6 +5,7 @@
>   *
>   */
>  #include <linux/clk.h>
> +#include <linux/of.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/spinlock.h>
>  
> @@ -610,12 +611,34 @@ get_crtc_primary(struct komeda_kms_dev *kms, struct komeda_crtc *crtc)
>  	return NULL;
>  }
>  
> +static int komeda_attach_bridge(struct device *dev,
> +				struct komeda_pipeline *pipe,
> +				struct drm_encoder *encoder)
> +{
> +	struct drm_bridge *bridge;
> +	int err;
> +
> +	bridge = devm_drm_of_get_bridge(dev, pipe->of_node,
> +					KOMEDA_OF_PORT_OUTPUT, 0);
> +	if (IS_ERR(bridge))
> +		return dev_err_probe(dev, PTR_ERR(bridge), "remote bridge not found for pipe: %s\n",
> +				     of_node_full_name(pipe->of_node));
> +
> +	err = drm_bridge_attach(encoder, bridge, NULL, 0);
> +	if (err)
> +		dev_err(dev, "bridge_attach() failed for pipe: %s\n",
> +			of_node_full_name(pipe->of_node));
> +
> +	return err;
> +}
> +
>  static int komeda_crtc_add(struct komeda_kms_dev *kms,
>  			   struct komeda_crtc *kcrtc)
>  {
>  	struct drm_crtc *crtc = &kcrtc->base;
>  	struct drm_device *base = &kms->base;
> -	struct drm_bridge *bridge;
> +	struct komeda_pipeline *pipe = kcrtc->master;
> +	struct drm_encoder *encoder = &kcrtc->encoder;
>  	int err;
>  
>  	err = drm_crtc_init_with_planes(base, crtc,
> @@ -626,27 +649,25 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>  
>  	drm_crtc_helper_add(crtc, &komeda_crtc_helper_funcs);
>  
> -	crtc->port = kcrtc->master->of_output_port;
> +	crtc->port = pipe->of_output_port;
>  
>  	/* Construct an encoder for each pipeline and attach it to the remote
>  	 * bridge
>  	 */
>  	kcrtc->encoder.possible_crtcs = drm_crtc_mask(crtc);
> -	err = drm_simple_encoder_init(base, &kcrtc->encoder,
> -				      DRM_MODE_ENCODER_TMDS);
> +	err = drm_simple_encoder_init(base, encoder, DRM_MODE_ENCODER_TMDS);
>  	if (err)
>  		return err;
>  
> -	bridge = devm_drm_of_get_bridge(base->dev, kcrtc->master->of_node,
> -					KOMEDA_OF_PORT_OUTPUT, 0);
> -	if (IS_ERR(bridge))
> -		return PTR_ERR(bridge);
> -
> -	err = drm_bridge_attach(&kcrtc->encoder, bridge, NULL, 0);
> +	if (pipe->of_output_links[0]) {
> +		err = komeda_attach_bridge(base->dev, pipe, encoder);
> +		if (err)
> +			return err;
> +	}
>  
>  	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
>  
> -	return err;
> +	return 0;
>  }
>  
>  int komeda_kms_add_crtcs(struct komeda_kms_dev *kms, struct komeda_dev *mdev)
> -- 
> 2.25.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 1/3] drm/arm/komeda: Fix komeda probe failing if there are no links in the secondary pipeline

[Faiz Abbas]

Hi,

On 3/6/24 7:13 PM, Liviu Dudau wrote:
> Hi Faiz,
>
> On Mon, Feb 19, 2024 at 03:39:13PM +0530, Faiz Abbas wrote:
>> Since commit f7936d6beda9 ("drm/arm/komeda: Remove component framework and
>> add a simple encoder"), the devm_drm_of_get_bridge() call happens
>> regardless of whether any remote nodes are available on the pipeline. Fix
>> this by moving the bridge attach to its own function and calling it
>> conditional on there being an output link.
>>
>> Fixes: f7936d6beda9 ("drm/arm/komeda: Remove component framework and add a simple encoder")
> I don't know what tree you're using, but the commit that you're quoting here is
> actually 4cfe5cc02e3f. I will fix it when I merge the patch.

Whoops. That would have been tough to fix. Thanks for catching it. I will be more careful in the future.

Thanks,

Faiz

[PATCH 2/3] drm/arm/komeda: Move pipeline prints to after the entire pipeline has been enabled

[Faiz Abbas]

The komeda driver prints a pretty verbose log in komeda_pipeline_dump()
detailing the components of each of the two pipelines. This gets printed
multiple times during boot as komeda EPROBE_DEFERs waiting for the
remote bridge drivers to come up. Move this log to after this has
happened indicating that the printed pipeline is actually completely up.

Signed-off-by: Faiz Abbas <faiz.abbas@arm.com>
---
 drivers/gpu/drm/arm/display/komeda/komeda_crtc.c     | 2 ++
 drivers/gpu/drm/arm/display/komeda/komeda_kms.h      | 1 +
 drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index b645c5998230..92ac09dc033b 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -667,6 +667,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
 
 	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
 
+	komeda_pipeline_dump(pipe);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
index a4048724564d..83e61c4080c2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h
@@ -191,5 +191,6 @@ void komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc,
 struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev);
 void komeda_kms_detach(struct komeda_kms_dev *kms);
 void komeda_kms_shutdown(struct komeda_kms_dev *kms);
+void komeda_pipeline_dump(struct komeda_pipeline *pipe);
 
 #endif /*_KOMEDA_KMS_H_*/
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
index 4b7d94961527..4b64ed9e9df5 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.c
@@ -9,6 +9,7 @@
 #include <drm/drm_print.h>
 
 #include "komeda_dev.h"
+#include "komeda_kms.h"
 #include "komeda_pipeline.h"
 
 /** komeda_pipeline_add - Add a pipeline to &komeda_dev */
@@ -246,7 +247,7 @@ static void komeda_component_dump(struct komeda_component *c)
 		  c->max_active_outputs, c->supported_outputs);
 }
 
-static void komeda_pipeline_dump(struct komeda_pipeline *pipe)
+void komeda_pipeline_dump(struct komeda_pipeline *pipe)
 {
 	struct komeda_component *c;
 	int id;
@@ -350,7 +351,6 @@ int komeda_assemble_pipelines(struct komeda_dev *mdev)
 		pipe = mdev->pipelines[i];
 
 		komeda_pipeline_assemble(pipe);
-		komeda_pipeline_dump(pipe);
 	}
 
 	return 0;
-- 
2.25.1

[PATCH 3/3] drm/arm/komeda: update DMA mask to 40 bits

[Faiz Abbas]

From: Amjad Ouled-Ameur <amjad.ouled-ameur@arm.com>

Each layer in the DPU has a 40-bit base address register, which indicates
start of frame buffer data for that layer. Komeda driver does not set
its DMA mask, which makes it 32-bit by default which does not use
the entire available possible supported by the DPU.

Update the DMA mask to align with DPU Architecture v1.0 spec.

Signed-off-by: Amjad Ouled-Ameur <amjad.ouled-ameur@arm.com>
Signed-off-by: Faiz Abbas <faiz.abbas@arm.com>
---
 drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index cc57ea4e13ae..fea5a4818f33 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -59,6 +59,10 @@ static int komeda_platform_probe(struct platform_device *pdev)
 	struct komeda_drv *mdrv;
 	int err;
 
+	err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
+	if (err)
+		return dev_err_probe(dev, err, "DMA mask error\n");
+
 	mdrv = devm_kzalloc(dev, sizeof(*mdrv), GFP_KERNEL);
 	if (!mdrv)
 		return -ENOMEM;
-- 
2.25.1

Re: [PATCH 0/3] Fixes for the komeda driver

[Liviu Dudau]

Hi Faiz,

On Mon, Feb 19, 2024 at 03:39:12PM +0530, Faiz Abbas wrote:
> The following patches add fixes to the komeda DPU driver.
> 
> Patch 1 fixes an issue where the crtc always expects both pipelines to
> always have remote nodes populated.
> 
> Patch 2 is a cosmetic fix that ensures komeda does not print verbose
> pipeline information unless the entire pipeline is actually up.
> 
> Patch 3 adds a 40 bit DMA mask for each pipeline.

Sorry for the delay in replying, I was on holiday last week.

Patch series looks good, I will push it into drm-misc-next-fixes at the end
of the week.

Best regards,
Liviu


> 
> Amjad Ouled-Ameur (1):
>   drm/arm/komeda: update DMA mask to 40 bits
> 
> Faiz Abbas (2):
>   drm/arm/komeda: Fix komeda probe failing if there are no links in the
>     secondary pipeline
>   drm/arm/komeda: Move pipeline prints to after the entire pipeline has
>     been enabled
> 
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  | 45 ++++++++++++++-----
>  .../gpu/drm/arm/display/komeda/komeda_drv.c   |  4 ++
>  .../gpu/drm/arm/display/komeda/komeda_kms.h   |  1 +
>  .../drm/arm/display/komeda/komeda_pipeline.c  |  4 +-
>  4 files changed, 41 insertions(+), 13 deletions(-)
> 
> -- 
> 2.25.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯