linux-kernel - Re: [01/30] drm: Introduce drm_bridge_init()

Open Source and information security mailing list archives

Message-ID: <20191203061212.GA16542@jamwan02-TSP300>
Date:   Tue, 3 Dec 2019 06:12:20 +0000
From:   "james qian wang (Arm Technology China)" <james.qian.wang@....com>
To:     Mihail Atanassov <Mihail.Atanassov@....com>,
        David Airlie <airlied@...ux.ie>, nd <nd@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [01/30] drm: Introduce drm_bridge_init()

On Mon, Dec 02, 2019 at 09:49:35AM +0100, Daniel Vetter wrote:
> On Mon, Dec 02, 2019 at 05:55:06AM +0000, james qian wang (Arm Technology China) wrote:
> > On Tue, Nov 26, 2019 at 01:15:59PM +0000, Mihail Atanassov wrote:
> > > A simple convenience function to initialize the struct drm_bridge.
> > >
> > > Signed-off-by: Mihail Atanassov <mihail.atanassov@....com>
> > > ---
> > >  drivers/gpu/drm/drm_bridge.c | 29 +++++++++++++++++++++++++++++
> > >  include/drm/drm_bridge.h     |  4 ++++
> > >  2 files changed, 33 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index cba537c99e43..cbe680aa6eac 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -89,6 +89,35 @@ void drm_bridge_remove(struct drm_bridge *bridge)
> > >  }
> > >  EXPORT_SYMBOL(drm_bridge_remove);
> > >
> > > +/**
> > > + * drm_bridge_init - initialise a drm_bridge structure
> > > + *
> > > + * @bridge: bridge control structure
> > > + * @funcs: control functions
> > > + * @dev: device
> > > + * @timings: timing specification for the bridge; optional (may be NULL)
> > > + * @driver_private: pointer to the bridge driver internal context (may be NULL)
> > > + */
> > > +void drm_bridge_init(struct drm_bridge *bridge, struct device *dev,
> > > +              const struct drm_bridge_funcs *funcs,
> > > +              const struct drm_bridge_timings *timings,
> > > +              void *driver_private)
> > > +{
> > > + WARN_ON(!funcs);
> > > +
> > > + bridge->dev = NULL;
> > > + bridge->encoder = NULL;
> > > + bridge->next = NULL;
> > > +
> > > +#ifdef CONFIG_OF
> > > + bridge->of_node = dev->of_node;
> > > +#endif
> > > + bridge->timings = timings;
> > > + bridge->funcs = funcs;
> > > + bridge->driver_private = driver_private;
> >
> > Can we directly put drm_bridge_add() here. then
> > - User always need to call bridge_init and add together.
> > - Consistent with others like drm_plane/crtc_init which directly has
> >   drm_mode_object_add() in it.
>
> Uh no, the trouble here is that drm_bridge_add should actually be called
> _register, because it publishes the bridge to the world. I think we even
> have a todo item to rename _add to _register ... Once that's done the
> bridge can't be changed anymore, all init code must have completed. So
> often you need a bit of code between _init() and _register().
>
> drm_mode_object_add is different since for mode objects it doesn't publish
> it to the world, that's done with drm_dev_register and
> drm_connector_register. drm_mode_object_add just does a bit of internal
> house keeping.
> -Daniel
>

Yes, the name _register() is more better.

And thank you for such detailed explanation.

Thanks
James

> >
> > James.
> > > +}
> > > +EXPORT_SYMBOL(drm_bridge_init);
> > > +
> > >  /**
> > >   * drm_bridge_attach - attach the bridge to an encoder's chain
> > >   *
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index c0a2286a81e9..d6d9d5301551 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -402,6 +402,10 @@ struct drm_bridge {
> > >
> > >  void drm_bridge_add(struct drm_bridge *bridge);
> > >  void drm_bridge_remove(struct drm_bridge *bridge);
> > > +void drm_bridge_init(struct drm_bridge *bridge, struct device *dev,
> > > +              const struct drm_bridge_funcs *funcs,
> > > +              const struct drm_bridge_timings *timings,
> > > +              void *driver_private);
> > >  struct drm_bridge *of_drm_find_bridge(struct device_node *np);
> > >  int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
> > >                 struct drm_bridge *previous);
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@...ts.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives