Skip to contentSkip to conversation

drm/sti: fix master bind bug for using component

PATCH In review Xinwei Kong 3 participants 7 messages Jul 14, 2015 – Jul 16, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drm/sti: fix master bind bug for using component

From: Xinwei Kong <[email protected]>

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.

---
 drivers/gpu/drm/sti/sti_tvout.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
drivers/gpu/drm/sti/sti_tvout.c +16 -1
index 5cc5311..293dfae 100644
@@ -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)
Signature · 6 lines
1.9.1

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw

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 <[email protected]>:

Quoted 43 lines

From: Xinwei Kong <[email protected]>

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.

---
 drivers/gpu/drm/sti/sti_tvout.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
drivers/gpu/drm/sti/sti_tvout.c +16 -2
index 5cc5311..293dfae 100644
@@ -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

Signature · 11 lines
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#2 lore raw
Quoted 11 lines · Benjamin Gaignard

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.

Signature · 6 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#3 lore raw

Thanks a lot Russell, I have now understand where was my mistake.

2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <[email protected]>:

Quoted 36 lines · Benjamin Gaignard
Quoted 11 lines

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.

Signature · 11 lines
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#4 lore raw

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

Quoted 43 lines · Benjamin Gaignard

Thanks a lot Russell, I have now understand where was my mistake.

2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <[email protected]>:

Quoted 36 lines · Benjamin Gaignard
Quoted 11 lines

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.

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

#5 lore raw

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 <[email protected]>:

Quoted 57 lines

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

Quoted 43 lines · Benjamin Gaignard

Thanks a lot Russell, I have now understand where was my mistake.

2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <[email protected]>:

Quoted 36 lines · Benjamin Gaignard
Quoted 11 lines

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.

Signature · 11 lines
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
#6 lore raw

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

Quoted 67 lines · Benjamin Gaignard

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 <[email protected]>:

Quoted 57 lines

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

Quoted 43 lines · Benjamin Gaignard

Thanks a lot Russell, I have now understand where was my mistake.

2015-07-15 12:42 GMT+02:00 Russell King - ARM Linux <[email protected]>:

Quoted 36 lines · Benjamin Gaignard
Quoted 11 lines

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.

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel