On Fri, Dec 15, 2023 at 07:47:07AM +0000, Biju Das wrote:
> Hi Maxime Ripard,
>
> > -----Original Message-----
> > From: Biju Das
> > Sent: Thursday, December 14, 2023 8:50 PM
> > Subject: RE: [PATCH v15 3/5] drm: renesas: Add RZ/G2L DU Support
> >
> > Hi Maxime Ripard,
> >
> >
> > > -----Original Message-----
> > > From: Biju Das
> > > Sent: Thursday, December 14, 2023 3:24 PM
> > > Subject: RE: [PATCH v15 3/5] drm: renesas: Add RZ/G2L DU Support
> > >
> > > >
> > > > > +
> > > > > + for (i = 0; i < num_planes; ++i) {
> > > > > + enum drm_plane_type type = i < num_crtcs
> > > > > + ? DRM_PLANE_TYPE_PRIMARY
> > > > > + : DRM_PLANE_TYPE_OVERLAY;
> > > > > + struct rzg2l_du_vsp_plane *plane = &vsp->planes[i];
> > > > > +
> > > > > + plane->vsp = vsp;
> > > > > + plane->index = i;
> > > > > + ret = drm_universal_plane_init(&rcdu->ddev, &plane-
> > >plane,
> > > > > + crtcs,
> > &rzg2l_du_vsp_plane_funcs,
> > > > > + rzg2l_du_vsp_formats,
> > > > > +
> > ARRAY_SIZE(rzg2l_du_vsp_formats),
> > > > > + NULL, type, NULL);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > >
> > > > you need to use drmm variant here too.
> > >
> > > I did rebased to latest drm_misc_next and I don't find the
> > > drmm_universal_plane_init()
> > >
> > > Can you please point me to the API?
> >
> > We cannot use drmm_universal_plane_alloc() in this architecture.
> >
> > rzg2l_du_vsps_init() stores the VSP pointer and pipe index from DT first.
> >
> > Then all the planes are created using rzg2l_du_vsp_init()
> >
> > CRTC uses VSP pointer and pipe_index to set the
> > plane(rzg2l_du_crtc_create()).
> >
> > CRTC->vsp->planes[rcrtc->vsp_pipe].plane
> >
>
> Thinking again, it is possible to use drmm_universal_plane_alloc() here
>
> Introduce a linked list [1] and use an API [2] to retrieve plane
> by matching vsp_pipe index against plane->index.
>
> With this we don't need drmm_kcalloc() any more.
>
> I will implement and test this and send v16, if there are no comments.
>
>
> [1]
> struct rzg2l_du_vsp_plane {
> struct drm_plane plane;
> struct rzg2l_du_vsp *vsp;
> unsigned int index;
> struct rzg2l_du_vsp_plane *next;
> };
>
> [2]
> struct drm_plane *rzg2l_du_vsp_get_drm_plane(struct rzg2l_du_crtc *crtc,
> unsigned int pipe_index);
Why do you need a linked list for? There's one already: the DRM device
will store all the planes registered in the DRM device, and you can
already iterate over all the planes with drm_for_each_plane, or
drm_for_each_plane_mask if you want to reduce the scope of the iterator.
Maxime
Attachment:
signature.asc
Description: PGP signature
