| Message ID | 55A4D648.6060404@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
The build order in Makefile hasn't been change so the bug doesn't occur... In addition of taking care of not changing build order in Makefile, I would like to understand what is the expected behavior of component framework when master call component_bind_all() before any component_add() calls. Should component bind function be called in component_add() ? Is up to component to detect that master is already bounded ? Russell can you tell us what to do in this case ? Regards, Benjamin 2015-07-14 11:28 GMT+02:00 Xinwei Kong <kong.kongxinwei@hisilicon.com>: > From: Xinwei Kong <kong.kongxinwei@hisilicon.com> > > This patch fix one bug which it can't call .bind function in > sti_hdmi.c and sti_hda.c file when changing the building sequence > (sti_hdmi.o?sti_hda.o) in Makefile file. This patch can prepare > it. > > Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com> > --- > drivers/gpu/drm/sti/sti_tvout.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c > index 5cc5311..293dfae 100644 > --- a/drivers/gpu/drm/sti/sti_tvout.c > +++ b/drivers/gpu/drm/sti/sti_tvout.c > @@ -683,7 +683,22 @@ static int compare_of(struct device *dev, void *data) > > static int sti_tvout_master_bind(struct device *dev) > { > - return 0; > + struct sti_tvout *tvout = dev_get_drvdata(dev); > + struct drm_device *drm_dev; > + int ret; > + > + if (!tvout->drm_dev) { > + DRM_ERROR("master bind is fail\n"); > + return 0; > + } > + > + drm_dev = tvout->drm_dev; > + > + ret = component_bind_all(dev, drm_dev); > + if (ret) > + sti_tvout_destroy_encoders(tvout); > + > + return ret; > } > > static void sti_tvout_master_unbind(struct device *dev) > -- > 1.9.1 >
On Wed, Jul 15, 2015 at 12:19:01PM +0200, Benjamin Gaignard wrote: > The build order in Makefile hasn't been change so the bug doesn't occur... > > In addition of taking care of not changing build order in Makefile, > I would like to understand what is the expected behavior of component > framework when > master call component_bind_all() before any component_add() calls. > > Should component bind function be called in component_add() ? > Is up to component to detect that master is already bounded ? > > Russell can you tell us what to do in this case ? I don't follow, and you certainly should never get into the situation you're alluding to (where the master is already bound but a component is not.) The way this should work is: - master and components register themselves into the component helper in a random order. - when the master registers, it gives the component helper a set of matches which it uses to determine which components are required. - when the component helper determines that all components and the master have been registered, it calls the master bind function. - the master bind function is responsible for the classical subsystem initialisation, calling component_bind_all() to cause the individual components bind() method to be called. So, you should never _ever_ be in the situation where initcall ordering matters, or where the master is already bound but a component hasn't registered.
Thanks a lot Russell, I have now understand where was my mistake. 2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Wed, Jul 15, 2015 at 12:19:01PM +0200, Benjamin Gaignard wrote: >> The build order in Makefile hasn't been change so the bug doesn't occur... >> >> In addition of taking care of not changing build order in Makefile, >> I would like to understand what is the expected behavior of component >> framework when >> master call component_bind_all() before any component_add() calls. >> >> Should component bind function be called in component_add() ? >> Is up to component to detect that master is already bounded ? >> >> Russell can you tell us what to do in this case ? > > I don't follow, and you certainly should never get into the situation > you're alluding to (where the master is already bound but a component > is not.) > > The way this should work is: > > - master and components register themselves into the component helper > in a random order. > - when the master registers, it gives the component helper a set of > matches which it uses to determine which components are required. > - when the component helper determines that all components and the > master have been registered, it calls the master bind function. > - the master bind function is responsible for the classical subsystem > initialisation, calling component_bind_all() to cause the individual > components bind() method to be called. > > So, you should never _ever_ be in the situation where initcall ordering > matters, or where the master is already bound but a component hasn't > registered. > > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net.
Thank you for Russel. It is Right, when this sti_tvout (component) finish executing component ".bind" function while sti_hdmi or sti_hda is not registered. the bug will occur . this patch will prepare this bug by calling master .bind of sti_tvout after sti_hdmi or sti_hda is register to finish binding sti_hdmi or sti_hda component, however, it also slove to bring the "drm_dev" struct into the sti_hdmi or sti_hda. Thank you Xinwei On 2015/7/15 21:17, Benjamin Gaignard wrote: > Thanks a lot Russell, I have now understand where was my mistake. > > > 2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: >> On Wed, Jul 15, 2015 at 12:19:01PM +0200, Benjamin Gaignard wrote: >>> The build order in Makefile hasn't been change so the bug doesn't occur... >>> >>> In addition of taking care of not changing build order in Makefile, >>> I would like to understand what is the expected behavior of component >>> framework when >>> master call component_bind_all() before any component_add() calls. >>> >>> Should component bind function be called in component_add() ? >>> Is up to component to detect that master is already bounded ? >>> >>> Russell can you tell us what to do in this case ? >> >> I don't follow, and you certainly should never get into the situation >> you're alluding to (where the master is already bound but a component >> is not.) >> >> The way this should work is: >> >> - master and components register themselves into the component helper >> in a random order. >> - when the master registers, it gives the component helper a set of >> matches which it uses to determine which components are required. >> - when the component helper determines that all components and the >> master have been registered, it calls the master bind function. >> - the master bind function is responsible for the classical subsystem >> initialisation, calling component_bind_all() to cause the individual >> components bind() method to be called. >> >> So, you should never _ever_ be in the situation where initcall ordering >> matters, or where the master is already bound but a component hasn't >> registered. >> >> -- >> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up >> according to speedtest.net. > > >
This patch isn't the good one, I send the fix here: http://lists.freedesktop.org/archives/dri-devel/2015-July/086568.html Regards, Benjamin 2015-07-16 3:13 GMT+02:00 Xinwei Kong <kong.kongxinwei@hisilicon.com>: > Thank you for Russel. > > It is Right, when this sti_tvout (component) finish executing component ".bind" > function while sti_hdmi or sti_hda is not registered. the bug will occur . > > this patch will prepare this bug by calling master .bind of sti_tvout after > sti_hdmi or sti_hda is register to finish binding sti_hdmi or sti_hda component, > however, it also slove to bring the "drm_dev" struct into the sti_hdmi or sti_hda. > > Thank you > Xinwei > > On 2015/7/15 21:17, Benjamin Gaignard wrote: >> Thanks a lot Russell, I have now understand where was my mistake. >> >> >> 2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: >>> On Wed, Jul 15, 2015 at 12:19:01PM +0200, Benjamin Gaignard wrote: >>>> The build order in Makefile hasn't been change so the bug doesn't occur... >>>> >>>> In addition of taking care of not changing build order in Makefile, >>>> I would like to understand what is the expected behavior of component >>>> framework when >>>> master call component_bind_all() before any component_add() calls. >>>> >>>> Should component bind function be called in component_add() ? >>>> Is up to component to detect that master is already bounded ? >>>> >>>> Russell can you tell us what to do in this case ? >>> >>> I don't follow, and you certainly should never get into the situation >>> you're alluding to (where the master is already bound but a component >>> is not.) >>> >>> The way this should work is: >>> >>> - master and components register themselves into the component helper >>> in a random order. >>> - when the master registers, it gives the component helper a set of >>> matches which it uses to determine which components are required. >>> - when the component helper determines that all components and the >>> master have been registered, it calls the master bind function. >>> - the master bind function is responsible for the classical subsystem >>> initialisation, calling component_bind_all() to cause the individual >>> components bind() method to be called. >>> >>> So, you should never _ever_ be in the situation where initcall ordering >>> matters, or where the master is already bound but a component hasn't >>> registered. >>> >>> -- >>> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up >>> according to speedtest.net. >> >> >> >
hi ben: your patch is ok, i don't know your hardware how to use. In this first code why use that style dts? If you detail research our patch, you will find that it can compatiable your fixing dts. If you don't approve our patch better, I will be glad to see you to slove this bug Thank you Xinwei On 2015/7/16 15:17, Benjamin Gaignard wrote: > This patch isn't the good one, I send the fix here: > http://lists.freedesktop.org/archives/dri-devel/2015-July/086568.html > > Regards, > Benjamin > > 2015-07-16 3:13 GMT+02:00 Xinwei Kong <kong.kongxinwei@hisilicon.com>: >> Thank you for Russel. >> >> It is Right, when this sti_tvout (component) finish executing component ".bind" >> function while sti_hdmi or sti_hda is not registered. the bug will occur . >> >> this patch will prepare this bug by calling master .bind of sti_tvout after >> sti_hdmi or sti_hda is register to finish binding sti_hdmi or sti_hda component, >> however, it also slove to bring the "drm_dev" struct into the sti_hdmi or sti_hda. >> >> Thank you >> Xinwei >> >> On 2015/7/15 21:17, Benjamin Gaignard wrote: >>> Thanks a lot Russell, I have now understand where was my mistake. >>> >>> >>> 2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: >>>> On Wed, Jul 15, 2015 at 12:19:01PM +0200, Benjamin Gaignard wrote: >>>>> The build order in Makefile hasn't been change so the bug doesn't occur... >>>>> >>>>> In addition of taking care of not changing build order in Makefile, >>>>> I would like to understand what is the expected behavior of component >>>>> framework when >>>>> master call component_bind_all() before any component_add() calls. >>>>> >>>>> Should component bind function be called in component_add() ? >>>>> Is up to component to detect that master is already bounded ? >>>>> >>>>> Russell can you tell us what to do in this case ? >>>> >>>> I don't follow, and you certainly should never get into the situation >>>> you're alluding to (where the master is already bound but a component >>>> is not.) >>>> >>>> The way this should work is: >>>> >>>> - master and components register themselves into the component helper >>>> in a random order. >>>> - when the master registers, it gives the component helper a set of >>>> matches which it uses to determine which components are required. >>>> - when the component helper determines that all components and the >>>> master have been registered, it calls the master bind function. >>>> - the master bind function is responsible for the classical subsystem >>>> initialisation, calling component_bind_all() to cause the individual >>>> components bind() method to be called. >>>> >>>> So, you should never _ever_ be in the situation where initcall ordering >>>> matters, or where the master is already bound but a component hasn't >>>> registered. >>>> >>>> -- >>>> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up >>>> according to speedtest.net. >>> >>> >>> >> > > >
diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index 5cc5311..293dfae 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -683,7 +683,22 @@ static int compare_of(struct device *dev, void *data) static int sti_tvout_master_bind(struct device *dev) { - return 0; + struct sti_tvout *tvout = dev_get_drvdata(dev); + struct drm_device *drm_dev; + int ret; + + if (!tvout->drm_dev) { + DRM_ERROR("master bind is fail\n"); + return 0; + } + + drm_dev = tvout->drm_dev; + + ret = component_bind_all(dev, drm_dev); + if (ret) + sti_tvout_destroy_encoders(tvout); + + return ret; } static void sti_tvout_master_unbind(struct device *dev)