On 29/01/25 - 12:00, José Expósito wrote: > Add a list of possible CRTCs to the encoder configuration and helpers to > attach and detach them. > > Now that the default configuration has its encoder and CRTC correctly > attached, configure the output following the configuration. > > Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx> > Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx> Co-developped-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx> Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx> Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx> [...] > diff --git a/drivers/gpu/drm/vkms/tests/vkms_config_test.c b/drivers/gpu/drm/vkms/tests/vkms_config_test.c [...] > +struct vkms_config_crtc ** > +vkms_config_encoder_get_possible_crtcs(struct vkms_config_encoder *encoder_cfg, > + size_t *out_length) > +{ > + struct vkms_config_crtc **array; > + struct vkms_config_crtc *possible_crtc; > + unsigned long idx; > + size_t length = 0; > + int n = 0; > + > + xa_for_each(&encoder_cfg->possible_crtcs, idx, possible_crtc) > + length++; > + > + if (length == 0) { > + *out_length = 0; > + return NULL; > + } > + > + array = kmalloc_array(length, sizeof(*array), GFP_KERNEL); > + if (!array) > + return ERR_PTR(-ENOMEM); > + > + xa_for_each(&encoder_cfg->possible_crtcs, idx, possible_crtc) { > + array[n] = possible_crtc; > + n++; > + } > + > + *out_length = length; > + return array; > +} Same as before, can we use an iterator? > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c [...] > @@ -98,22 +143,8 @@ int vkms_output_init(struct vkms_device *vkmsdev) > goto err_free; > } > > - encoder = drmm_kzalloc(dev, sizeof(*encoder), GFP_KERNEL); > - if (!encoder) { > - DRM_ERROR("Failed to allocate encoder\n"); > - ret = -ENOMEM; > - goto err_free; > - } > - ret = drmm_encoder_init(dev, encoder, NULL, > - DRM_MODE_ENCODER_VIRTUAL, NULL); > - if (ret) { > - DRM_ERROR("Failed to init encoder\n"); > - goto err_free; > - } > - encoder->possible_crtcs = drm_crtc_mask(&crtc_cfgs[0]->crtc->crtc); > - > /* Attach the encoder and the connector */ > - ret = drm_connector_attach_encoder(&connector->base, encoder); > + ret = drm_connector_attach_encoder(&connector->base, encoder_cfgs[0]->encoder); Why do you only attach the first encoder? Can't we attach all of them? [...]