[PATCH v2 0/2] drm/komeda: Fix GLB_CORE_ID parsing and initialize encoder clone masks

[PATCH v2 0/2] drm/komeda: Fix GLB_CORE_ID parsing and initialize encoder clone masks

[Raveendra Talabattula]

Komeda currently decodes GLB_CORE_ID with bit definitions that do not
match the hardware register layout, which leads to incorrect hardware
version reporting.

Patch 1 fixes the GLB_CORE_ID field definitions so the reported product
version matches the hardware specification.

DRM clone-mask validation can reject Komeda display + writeback
configurations because the encoder pair attached to one CRTC does not
advertise the required reciprocal possible_clones relationship.

Patch 2 sets the display/writeback clone masks when the writeback
connector is created, making the clone relationship explicit for the
encoder pair attached to that CRTC.

Changes in v2:
- Patch 1: add a short GLB_CORE_ID layout comment and drop the redundant
  product-id mask and shift-by-zero change noted in review.
- Patch 2: replace the global encoder mask initialization with per-CRTC
  display/writeback clone-mask setup.
- Patch 2: include each encoder's own bit in possible_clones.
- Update patch 2 subject and cover letter text to match the localized fix.

Cc: liviu.dudau@arm.com
Cc: maarten.lankhorst@linux.intel.com
Cc: mripard@kernel.org
Cc: tzimmermann@suse.de
Cc: airlied@gmail.com
Cc: simona@ffwll.ch
Cc: james.qian.wang@arm.com
Cc: asad.malik@arm.com
Cc: vincenzo.frascino@arm.com
Cc: nayden.kanchev@arm.com
Cc: charvi.mehta@arm.com
Signed-off-by: Raveendra Talabattula <raveendra.talabattula@arm.com>

Raveendra Talabattula (1):
  drm/komeda: Fix bits parsing of GLB_CORE_ID

Vincenzo Frascino (1):
  drm/komeda: Set display/writeback clone masks

 .../drm/arm/display/include/malidp_product.h  | 10 +++++++---
 .../arm/display/komeda/komeda_wb_connector.c  | 33 +++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] drm/komeda: Fix bits parsing of GLB_CORE_ID

[Raveendra Talabattula]

GLB_CORE_ID contains the product and version information returned by the
hardware. The current macros parses VERSION_MINOR as a 4-bit field and
VERSION_STATUS as an 8-bit field, which does not match the register
layout. As a result, the driver reports an incorrect version number.

Fix the parsing macros to match the hardware specification:
- VERSION_MINOR is 8 bits at [11:4]
- VERSION_STATUS is 4 bits at [3:0]

Fixes: bd628c1bed79 ("drm/komeda: komeda_dev/pipeline/component definition and initialzation")
Signed-off-by: Raveendra Talabattula <raveendra.talabattula@arm.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
---
 drivers/gpu/drm/arm/display/include/malidp_product.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/include/malidp_product.h b/drivers/gpu/drm/arm/display/include/malidp_product.h
index 6f954bcdf40e..f9a3ee7ba4b7 100644
--- a/drivers/gpu/drm/arm/display/include/malidp_product.h
+++ b/drivers/gpu/drm/arm/display/include/malidp_product.h
@@ -8,14 +8,18 @@
 #define _MALIDP_PRODUCT_H_
 
 /* Product identification */
+/* GLB_CORE_ID fields as per HW specification:
+ * MINOR is 8 bits ([11:4]) and STATUS is 4 bits ([3:0]).
+ * Update masks/shifts accordingly.
+ */
 #define MALIDP_CORE_ID(__product, __major, __minor, __status) \
 	((((__product) & 0xFFFF) << 16) | (((__major) & 0xF) << 12) | \
-	(((__minor) & 0xF) << 8) | ((__status) & 0xFF))
+	(((__minor) & 0xFF) << 4) | ((__status) & 0xF))
 
 #define MALIDP_CORE_ID_PRODUCT_ID(__core_id) ((__u32)(__core_id) >> 16)
 #define MALIDP_CORE_ID_MAJOR(__core_id)      (((__u32)(__core_id) >> 12) & 0xF)
-#define MALIDP_CORE_ID_MINOR(__core_id)      (((__u32)(__core_id) >> 8) & 0xF)
-#define MALIDP_CORE_ID_STATUS(__core_id)     (((__u32)(__core_id)) & 0xFF)
+#define MALIDP_CORE_ID_MINOR(__core_id)      (((__u32)(__core_id) >>  4) & 0xFF)
+#define MALIDP_CORE_ID_STATUS(__core_id)     ((__u32)(__core_id) & 0xF)
 
 /* Mali-display product IDs */
 #define MALIDP_D71_PRODUCT_ID	0x0071
-- 
2.43.0

[PATCH v2 2/2] drm/komeda: Set display/writeback clone masks

[Raveendra Talabattula]

From: Vincenzo Frascino <vincenzo.frascino@arm.com>

Since commit 41b4b11da0215 ("drm: Add valid clones check"),
all encoders attached to the same CRTC must advertise
each other through possible_clones.

Komeda creates one writeback connector per CRTC. Allow the writeback
encoder to be cloned only with encoders that can drive the same CRTC,
and update those encoders reciprocally.

Do this when the writeback connector is created rather than modifying
every encoder registered with the DRM device from komeda_kms_attach().
This leaves encoders associated with other CRTCs untouched.

Signed-off-by: Asad Malik <asad.malik@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Co-developed-by: Raveendra Talabattula <raveendra.talabattula@arm.com>
Signed-off-by: Raveendra Talabattula <raveendra.talabattula@arm.com>
---
 .../arm/display/komeda/komeda_wb_connector.c  | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
index 41cc3e080dc9..4872253a2449 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
@@ -143,12 +143,17 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 	struct komeda_wb_connector *kwb_conn;
 	struct drm_writeback_connector *wb_conn;
 	struct drm_display_info *info;
+	struct drm_encoder *encoder;
+	struct drm_encoder *wb_encoder;
 	u32 *formats, n_formats = 0;
+	u32 crtc_mask;
 	int err;
 
 	if (!kcrtc->master->wb_layer)
 		return 0;
 
+	crtc_mask = drm_crtc_mask(&kcrtc->base);
+
 	kwb_conn = kzalloc_obj(*kwb_conn);
 	if (!kwb_conn)
 		return -ENOMEM;
@@ -176,6 +181,34 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
 		return err;
 	}
 
+	wb_encoder = &wb_conn->encoder;
+
+	/*
+	 * The writeback connector is associated with a single CRTC. Make its
+	 * encoder clone-compatible only with encoders that can drive that CRTC.
+	 *
+	 * possible_clones must contain the encoder's own bit whenever it is
+	 * non-zero. Add both the encoder itself and the writeback encoder when
+	 * updating the reciprocal clone relationship.
+	 */
+	wb_encoder->possible_clones = drm_encoder_mask(wb_encoder);
+
+	drm_for_each_encoder(encoder, &kms->base) {
+		u32 encoder_mask;
+
+		if (encoder == wb_encoder)
+			continue;
+
+		if (!(encoder->possible_crtcs & crtc_mask))
+			continue;
+
+		encoder_mask = drm_encoder_mask(encoder);
+
+		wb_encoder->possible_clones |= encoder_mask;
+		encoder->possible_clones |= encoder_mask |
+					    drm_encoder_mask(wb_encoder);
+	}
+
 	drm_connector_helper_add(&wb_conn->base, &komeda_wb_conn_helper_funcs);
 
 	info = &kwb_conn->base.base.display_info;
-- 
2.43.0

Re: [PATCH v2 2/2] drm/komeda: Set display/writeback clone masks

[Liviu Dudau]

On Mon, Aug 03, 2026 at 03:21:22PM +0100, Raveendra Talabattula wrote:
> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
> 
> Since commit 41b4b11da0215 ("drm: Add valid clones check"),
> all encoders attached to the same CRTC must advertise
> each other through possible_clones.
> 
> Komeda creates one writeback connector per CRTC. Allow the writeback
> encoder to be cloned only with encoders that can drive the same CRTC,
> and update those encoders reciprocally.
> 
> Do this when the writeback connector is created rather than modifying
> every encoder registered with the DRM device from komeda_kms_attach().
> This leaves encoders associated with other CRTCs untouched.

If you don't mind I will drop the paragraph above when I commit the patch
as it makes reference to code that has not been merged so likely
to create confusion. I think the comment in the code is sufficient.

With that,

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Will push this to drm-misc-next in the next couple of days.

Best regards,
Liviu

> 
> Signed-off-by: Asad Malik <asad.malik@arm.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Co-developed-by: Raveendra Talabattula <raveendra.talabattula@arm.com>
> Signed-off-by: Raveendra Talabattula <raveendra.talabattula@arm.com>
> ---
>  .../arm/display/komeda/komeda_wb_connector.c  | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> index 41cc3e080dc9..4872253a2449 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c
> @@ -143,12 +143,17 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
>  	struct komeda_wb_connector *kwb_conn;
>  	struct drm_writeback_connector *wb_conn;
>  	struct drm_display_info *info;
> +	struct drm_encoder *encoder;
> +	struct drm_encoder *wb_encoder;
>  	u32 *formats, n_formats = 0;
> +	u32 crtc_mask;
>  	int err;
>  
>  	if (!kcrtc->master->wb_layer)
>  		return 0;
>  
> +	crtc_mask = drm_crtc_mask(&kcrtc->base);
> +
>  	kwb_conn = kzalloc_obj(*kwb_conn);
>  	if (!kwb_conn)
>  		return -ENOMEM;
> @@ -176,6 +181,34 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms,
>  		return err;
>  	}
>  
> +	wb_encoder = &wb_conn->encoder;
> +
> +	/*
> +	 * The writeback connector is associated with a single CRTC. Make its
> +	 * encoder clone-compatible only with encoders that can drive that CRTC.
> +	 *
> +	 * possible_clones must contain the encoder's own bit whenever it is
> +	 * non-zero. Add both the encoder itself and the writeback encoder when
> +	 * updating the reciprocal clone relationship.
> +	 */
> +	wb_encoder->possible_clones = drm_encoder_mask(wb_encoder);
> +
> +	drm_for_each_encoder(encoder, &kms->base) {
> +		u32 encoder_mask;
> +
> +		if (encoder == wb_encoder)
> +			continue;
> +
> +		if (!(encoder->possible_crtcs & crtc_mask))
> +			continue;
> +
> +		encoder_mask = drm_encoder_mask(encoder);
> +
> +		wb_encoder->possible_clones |= encoder_mask;
> +		encoder->possible_clones |= encoder_mask |
> +					    drm_encoder_mask(wb_encoder);
> +	}
> +
>  	drm_connector_helper_add(&wb_conn->base, &komeda_wb_conn_helper_funcs);
>  
>  	info = &kwb_conn->base.base.display_info;
> -- 
> 2.43.0
> 

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

Re: [PATCH v2 2/2] drm/komeda: Set display/writeback clone masks

[Vincenzo Frascino]

On 03/08/2026 15:46, Liviu Dudau wrote:
> On Mon, Aug 03, 2026 at 03:21:22PM +0100, Raveendra Talabattula wrote:
>> From: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>
>> Since commit 41b4b11da0215 ("drm: Add valid clones check"),
>> all encoders attached to the same CRTC must advertise
>> each other through possible_clones.
>>
>> Komeda creates one writeback connector per CRTC. Allow the writeback
>> encoder to be cloned only with encoders that can drive the same CRTC,
>> and update those encoders reciprocally.
>>
>> Do this when the writeback connector is created rather than modifying
>> every encoder registered with the DRM device from komeda_kms_attach().
>> This leaves encoders associated with other CRTCs untouched.
> 
> If you don't mind I will drop the paragraph above when I commit the patch
> as it makes reference to code that has not been merged so likely
> to create confusion. I think the comment in the code is sufficient.
> 

Of course Liviu, I mentioned it to make sure that it was clear I looked at the
code you mentioned in review.

We are happy to address the relevant changes once the new series gets merged.

Thanks,
Vincenzo

> With that,
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> 
> Will push this to drm-misc-next in the next couple of days.
> 
> Best regards,
> Liviu
> ...