Re: [PATCH] drm: bridge: tc358764: Use drm panel_bridge API

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, Dec 13, 2021 at 06:09:23PM +0530, Jagan Teki wrote:
> Hi Laurent,
> 
> On Mon, Dec 13, 2021 at 6:02 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Jagan,
> >
> > Thank you for the patch.
> >
> > On Mon, Dec 13, 2021 at 05:49:29PM +0530, Jagan Teki wrote:
> > > Replace the manual panel handling code by a drm panel_bridge via
> > > devm_drm_of_get_bridge().
> > >
> > > Adding panel_bridge handling,
> > >
> > > - Drops drm_connector and related operations as drm_bridge_attach
> > >   creates connector during attachment.
> > >
> > > - Drops panel pointer and panel healpers.
> > >
> > > This simplifies the driver and allows all components in the display
> > > pipeline to be treated as bridges.
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  drivers/gpu/drm/bridge/tc358764.c | 99 ++-----------------------------
> > >  1 file changed, 6 insertions(+), 93 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
> > > index c1e35bdf9232..28480bdc4287 100644
> > > --- a/drivers/gpu/drm/bridge/tc358764.c
> > > +++ b/drivers/gpu/drm/bridge/tc358764.c
> > > @@ -153,10 +153,9 @@ static const char * const tc358764_supplies[] = {
> > >  struct tc358764 {
> > >       struct device *dev;
> > >       struct drm_bridge bridge;
> > > -     struct drm_connector connector;
> > > +     struct drm_bridge *panel_bridge;
> >
> > s/panel_bridge/next_bridge/ as it may not be a panel.
> 
> Sometime, I'm a strong believer of my own notation (I may be wrong)
> based on my understanding. This is downstream bridge and the only
> option it to connect is panel and panel in bridge terminology are
> treated as panel_bridge. This is the reason I have used panel_bridge.
> next_bridge notation will be used if the bridge connected to any
> downstream bridge, like we can use next_bridge notation in host bridge
> drivers as host bridge can be an option of connecting downstream
> bridge or panel.

The downstream bridge doesn't have to be a DSI panel, it could be an
LVDS-to-DPI bridge for instance, or an LVDS-to-HDMI encoder.

> This is what I understood so-far with DRM bridges. May be you can
> correct if I'm wrong.
> 
> > >       struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
> > >       struct gpio_desc *gpio_reset;
> > > -     struct drm_panel *panel;
> >
> > Are there #includes that you can drop ?
> 
> I think, yes. I will update it in v2.

-- 
Regards,

Laurent Pinchart