| Message ID | 20191126131541.47393-2-mihail.atanassov@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: Add device links for lifetime control | expand |
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@arm.com> The commit message here leaves figuring out why we need this to the reader. Reading ahead the reasons seems to be to roll out bridge->dev setting for everyone, so that we can set the device_link. Please explain that in the commit message so the patch is properly motivated. > --- > 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) Please also sprinkle some links to this new function to relevant places, I'd add at least: "Drivers should call drm_bridge_init() first." to the kerneldoc for drm_bridge_add. drm_bridge_add should also mention drm_bridge_remove as the undo function. And perhaps a longer paragraph to &struct drm_bridge: "Bridge drivers should call drm_bridge_init() to initialized a bridge driver, and then register it with drm_bridge_add(). "Users of bridges link a bridge driver into their overall display output pipeline by calling drm_bridge_attach()." > + */ > +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; Given that the goal here is to get bridge->dev set up, why not WARN_ON(!dev); bridge->dev = dev; That should help us to really move forward with all this. -Daniel > + 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; > +} > +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); > -- > 2.23.0 >
Hi Daniel, Thanks for the quick review. On Tuesday, 26 November 2019 14:26:10 GMT Daniel Vetter 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@arm.com> > > The commit message here leaves figuring out why we need this to the > reader. Reading ahead the reasons seems to be to roll out bridge->dev > setting for everyone, so that we can set the device_link. Please explain > that in the commit message so the patch is properly motivated. Ack, but with one caveat: bridge->dev is the struct drm_device that is the bridge client, we need to add a bridge->device (patch 29 in this series) which is the struct device that will manage the bridge lifetime. > > > --- > > 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) > > Please also sprinkle some links to this new function to relevant places, > I'd add at least: > > "Drivers should call drm_bridge_init() first." to the kerneldoc for > drm_bridge_add. drm_bridge_add should also mention drm_bridge_remove as > the undo function. > > And perhaps a longer paragraph to &struct drm_bridge: > > "Bridge drivers should call drm_bridge_init() to initialized a bridge > driver, and then register it with drm_bridge_add(). > > "Users of bridges link a bridge driver into their overall display output > pipeline by calling drm_bridge_attach()." Will do. > > > + */ > > +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; > > Given that the goal here is to get bridge->dev set up, why not > > WARN_ON(!dev); > bridge->dev = dev; See above struct device vs struct drm_device. I add a bridge->device = dev; in patch 29, which takes care of that. I skipped the warn since there's a dereference of dev, but I now realized it's behind CONFIG_OF, so I'll add it in for v2. Yes, 'device' isn't the best of names, but I took Russell's patch almost as-is, I didn't have any better ideas for bikeshedding. > > That should help us to really move forward with all this. > -Daniel > > > + 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; > > +} > > +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); > >
On Tue, Nov 26, 2019 at 4:55 PM Mihail Atanassov <Mihail.Atanassov@arm.com> wrote: > > Hi Daniel, > > Thanks for the quick review. > > On Tuesday, 26 November 2019 14:26:10 GMT Daniel Vetter 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@arm.com> > > > > The commit message here leaves figuring out why we need this to the > > reader. Reading ahead the reasons seems to be to roll out bridge->dev > > setting for everyone, so that we can set the device_link. Please explain > > that in the commit message so the patch is properly motivated. > > Ack, but with one caveat: bridge->dev is the struct drm_device that is > the bridge client, we need to add a bridge->device (patch 29 in this > series) which is the struct device that will manage the bridge lifetime. Ah yes, ->dev is for drm_bridge_attach. > > > > > --- > > > 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) > > > > Please also sprinkle some links to this new function to relevant places, > > I'd add at least: > > > > "Drivers should call drm_bridge_init() first." to the kerneldoc for > > drm_bridge_add. drm_bridge_add should also mention drm_bridge_remove as > > the undo function. > > > > And perhaps a longer paragraph to &struct drm_bridge: > > > > "Bridge drivers should call drm_bridge_init() to initialized a bridge > > driver, and then register it with drm_bridge_add(). > > > > "Users of bridges link a bridge driver into their overall display output > > pipeline by calling drm_bridge_attach()." > > Will do. > > > > > > + */ > > > +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; > > > > Given that the goal here is to get bridge->dev set up, why not > > > > WARN_ON(!dev); > > bridge->dev = dev; > > See above struct device vs struct drm_device. I add a > > bridge->device = dev; > > in patch 29, which takes care of that. I skipped the warn since > there's a dereference of dev, but I now realized it's behind CONFIG_OF, > so I'll add it in for v2. Ok, sounds good. Having the WARN_ON in patch 1 should also help making sure all the conversion patches dtrt (and any future users). -Daniel > Yes, 'device' isn't the best of names, but I took Russell's patch > almost as-is, I didn't have any better ideas for bikeshedding. > > > > > That should help us to really move forward with all this. > > -Daniel > > > > > + 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; > > > +} > > > +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); > > > > > > > -- > Mihail > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Hi Mihail. > Ack, but with one caveat: bridge->dev is the struct drm_device that is > the bridge client, we need to add a bridge->device (patch 29 in this > series) which is the struct device that will manage the bridge lifetime. Other places uses the variable name "drm" for a drm_device. This is less confusion than the "dev" name. It seems a recent trend to use the variable name "drm" so you can find a lot of places using "dev". bike-shedding - but also about readability. Sam
On Tuesday, 26 November 2019 19:24:45 GMT Sam Ravnborg wrote: > Hi Mihail. Hi Sam, > > > Ack, but with one caveat: bridge->dev is the struct drm_device that is > > the bridge client, we need to add a bridge->device (patch 29 in this > > series) which is the struct device that will manage the bridge lifetime. > Other places uses the variable name "drm" for a drm_device. > This is less confusion than the "dev" name. > > It seems a recent trend to use the variable name "drm" so you can find a > lot of places using "dev". > > bike-shedding - but also about readability. > > Sam > I'm okay with the idea, I can do a follow-up patch or series for the rename; I expect it would be a bit hefty to do it prior to this. @Daniel, thoughts on s/bridge.dev/bridge.drm/ and s/bridge.device/bridge.dev/ after this series?
On Wed, Nov 27, 2019 at 11:05:56AM +0000, Mihail Atanassov wrote: > On Tuesday, 26 November 2019 19:24:45 GMT Sam Ravnborg wrote: > > Hi Mihail. > > Hi Sam, > > > > > > Ack, but with one caveat: bridge->dev is the struct drm_device that is > > > the bridge client, we need to add a bridge->device (patch 29 in this > > > series) which is the struct device that will manage the bridge lifetime. > > Other places uses the variable name "drm" for a drm_device. > > This is less confusion than the "dev" name. > > > > It seems a recent trend to use the variable name "drm" so you can find a > > lot of places using "dev". > > > > bike-shedding - but also about readability. > > > > Sam > > > > I'm okay with the idea, I can do a follow-up patch or series for the > rename; I expect it would be a bit hefty to do it prior to this. > > @Daniel, thoughts on s/bridge.dev/bridge.drm/ and > s/bridge.device/bridge.dev/ after this series? I'm firmly in the "no opionon" on this. -Daniel
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@arm.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. 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);
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@arm.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 > > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@arm.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@lists.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.
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; +} +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);
A simple convenience function to initialize the struct drm_bridge. Signed-off-by: Mihail Atanassov <mihail.atanassov@arm.com> --- drivers/gpu/drm/drm_bridge.c | 29 +++++++++++++++++++++++++++++ include/drm/drm_bridge.h | 4 ++++ 2 files changed, 33 insertions(+)