<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Fri, Mar 14, 2025 at 8:27 AM Luca Ceresoli <<a href="mailto:luca.ceresoli@bootlin.com">luca.ceresoli@bootlin.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Anusha,<br>
<br>
On Thu, 13 Mar 2025 16:34:45 -0400<br>
Anusha Srivatsa <<a href="mailto:asrivats@redhat.com" target="_blank">asrivats@redhat.com</a>> wrote:<br>
<br>
> > > +void *__devm_drm_panel_alloc(struct device *dev, size_t size, size_t  <br>
> > offset,  <br>
> > > +                          const struct drm_panel_funcs *funcs)<br>
> > > +{<br>
> > > +     void *container;<br>
> > > +     struct drm_panel *panel;<br>
> > > +     int err;<br>
> > > +<br>
> > > +     if (!funcs) {<br>
> > > +             dev_warn(dev, "Missing funcs pointer\n");<br>
> > > +             return ERR_PTR(-EINVAL);<br>
> > > +     }<br>
> > > +<br>
> > > +     container = kzalloc(size, GFP_KERNEL);<br>
> > > +     if (!container)<br>
> > > +             return ERR_PTR(-ENOMEM);<br>
> > > +<br>
> > > +     panel = container + offset;<br>
> > > +     panel->container_offset = offset;<br>
> > > +     panel->funcs = funcs;<br>
> > > +     kref_init(&panel->refcount);<br>
> > > +<br>
> > > +     err = devm_add_action_or_reset(dev, drm_panel_put_void, panel);<br>
> > > +     if (err)<br>
> > > +             return ERR_PTR(err);<br>
> > > +<br>
> > > +     drm_panel_init(panel, dev, funcs, panel->connector_type);  <br>
> ><br>
> > panel->connector_type here is uninitialized. You are passing<br>
> > panel->connector_type to drm_panel_init(), which will then copy it into<br>
> > panel->connector_type itself.<br>
> ><br>
> > So you mean I pass connector_type from the driver calling the helper, so  <br>
> there is access to the connector type here?<br>
<br>
I'm not a panel expert, but I think it makes sense that to create the<br>
panel you need to know the connection type, and that is what Maxime<br>
suggested.<br>
<br>
> > > +     /**<br>
> > > +      * @container_offset: Offset of this struct within the container<br>
> > > +      * struct embedding it. Used for refcounted panels to free the<br>
> > > +      * embeddeing struct when the refcount drops to zero.<br>
> > > +      */<br>
> > > +     size_t container_offset;  <br>
> ><br>
> > While storing the offset obviously works, and that's what I had<br>
> > implemented in my latest bridge refcounting series, after some<br>
> > discussion with Maxime we agreed storing a container pointer instead of<br>
> > the offset is cleaner. I think it would be good here as well.<br>
> ><br>
> > See:<br>
> > <a href="https://lore.kernel.org/lkml/20250227-macho-convivial-tody-cea7dc@houat/" rel="noreferrer" target="_blank">https://lore.kernel.org/lkml/20250227-macho-convivial-tody-cea7dc@houat/</a><br>
> >  <br>
> <br>
> so just void *container instead of size_t container_offset.<br>
<br>
Exactly. You can have a look at the patch I sent earlier today:<br>
<a href="https://lore.kernel.org/lkml/20250314-drm-bridge-refcount-v7-2-152571f8c694@bootlin.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/lkml/20250314-drm-bridge-refcount-v7-2-152571f8c694@bootlin.com/</a><br>
<br></blockquote><div><br></div><div>This helps. Thanks</div><div><br></div><div>Anusha <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Luca<br>
<br>
-- <br>
Luca Ceresoli, Bootlin<br>
Embedded Linux and Kernel engineering<br>
<a href="https://bootlin.com" rel="noreferrer" target="_blank">https://bootlin.com</a><br>
<br>
</blockquote></div></div>