Re: [PATCH 2/2] drm/komeda: Initialize encoder possible_clones

[Liviu Dudau <liviu.dudau@arm.com>]

On Tue, Jul 21, 2026 at 02:52:27PM +0100, Raveendra Talabattula wrote:
> Komeda leaves encoder->possible_clones unset, so it stays at 0 for every
> encoder. When userspace asks for writeback, the DRM atomic checks

Can you tell me more about this? What are you trying to do with the writeback?

Please note that there is a patch series that changes the way encoders get
created for the writeback connectors, so that can potentially affect your
use case.

> reject the configuration because no encoder is marked as clone-compatible,
> leading to errors such as:
> 
> 	crtc96 failed valid clone check for mask 0x5

The error you're seeing here is due to the encoder having a non-zero "possible_clones"
which shows there is an error in your setup earlier and nothing to do with komeda.

> 
> Komeda does not impose per-encoder clone restrictions, [...]

Komeda doesn't care about the encoders at all as it is meant to be agnostic
to whatever encoder is used.

> [...] so initialize
> possible_clones for all registered encoders to the full encoder mask
> after encoder creation. This fixes writeback validation.
> 
> Signed-off-by: Asad Malik <asad.malik@arm.com>
> Signed-off-by: Raveendra Talabattula <raveendra.talabattula@arm.com>
> ---
>  .../gpu/drm/arm/display/komeda/komeda_kms.c   | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> index 6ed504099188..0dcc8c05e86b 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c
> @@ -276,7 +276,10 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev)
>  {
>  	struct komeda_kms_dev *kms;
>  	struct drm_device *drm;
> +	struct drm_encoder *encoder;
>  	int err;
> +	/* Bitmap of all encoders, assigned to each encoder's possible_clones. */
> +	u32 clone_mask = 0;
>  
>  	kms = devm_drm_dev_alloc(mdev->dev, &komeda_kms_driver,
>  				 struct komeda_kms_dev, base);
> @@ -311,6 +314,23 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev)
>  
>  	drm_mode_config_reset(drm);
>  
> +	/*
> +	 * Build the full possible_clones mask once. drm_encoder_index()
> +	 * returns the bit position assigned to each encoder and BIT() converts
> +	 * that index into the corresponding mask value.
> +	 *
> +	 * Komeda does not have per-encoder clone restrictions, so every encoder
> +	 * gets the same mask and is advertised as clone-compatible with all
> +	 * other registered encoders.
> +	 */
> +	drm_for_each_encoder(encoder, drm) {
> +		clone_mask |= BIT(drm_encoder_index(encoder));
> +	}
> +
> +	drm_for_each_encoder(encoder, drm) {
> +		encoder->possible_clones = clone_mask;
> +	}

You're modifying all the encoders in the system here which is not the right thing.

Best regards,
Liviu

> +
>  	err = devm_request_irq(drm->dev, mdev->irq,
>  			       komeda_kms_irq_handler, IRQF_SHARED,
>  			       drm->driver->name, drm);
> -- 
> 2.43.0
> 

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