component: Support masters with no subcomponents

[PATCH] component: Support masters with no subcomponents

Posted by Herman van Hazendonk 2 weeks ago

This happens in the MSM DRM driver when it is used
without any subcomponents, which is a special corner
case.

If the MDP4 is used with nothing but the LVDS display,
we get this problem that no components are found since
the LVDS is part of the MDP4 itself.

We cannot use a NULL match, so create a dummy match
with no components for this case so the driver will
still probe nicely without adding a secondary
complicated probe() path to the MSM DRM driver.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
This happens in the MSM DRM driver when it is used
without any subcomponents, which is a special corner
case.

If the MDP4 is used with nothing but the LVDS display,
we get this problem that no components are found since
the LVDS is part of the MDP4 itself.

We cannot use a NULL match, so create a dummy match
with no components for this case so the driver will
still probe nicely without adding a secondary
complicated probe() path to the MSM DRM driver.
---
 drivers/base/component.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 741497324d78..bb79e7a77bb0 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -497,6 +497,10 @@ static void free_aggregate_device(struct aggregate_device *adev)
 	kfree(adev);
 }
 
+static struct component_match dummy_match = {
+	.num = 0,
+};
+
 /**
  * component_master_add_with_match - register an aggregate driver
  * @parent: parent device of the aggregate driver
@@ -516,6 +520,9 @@ int component_master_add_with_match(struct device *parent,
 	struct aggregate_device *adev;
 	int ret;
 
+	if (!match)
+		match = &dummy_match;
+
 	/* Reallocate the match array for its true size */
 	ret = component_match_realloc(match, match->num);
 	if (ret)

---
base-commit: 96fca68c4fbf77a8185eb10f7557e23352732ea2
change-id: 20240417-component-dummy-a9aae5ac7234

Best regards,
-- 
Herman van Hazendonk <github.com@herrie.org>

Re: [PATCH] component: Support masters with no subcomponents

Posted by Linus Walleij 2 weeks ago

Hi Herman,

thanks for your patch!

Do you actually have this working on real hardware? I never
submitted this patch because I could not get the hardware
working.

I was hoping for smarter people (Dmitry Baryshkov...) to step
in and help out to actually make it work before submitting
patches.

Yours,
Linus Walleij

Re: [PATCH] component: Support masters with no subcomponents

Posted by Dmitry Baryshkov 2 weeks ago

On Wed, Apr 17, 2024 at 01:39:16PM +0200, Linus Walleij wrote:
> Hi Herman,
> 
> thanks for your patch!
> 
> Do you actually have this working on real hardware? I never
> submitted this patch because I could not get the hardware
> working.
> 
> I was hoping for smarter people (Dmitry Baryshkov...) to step
> in and help out to actually make it work before submitting
> patches.

I have LVDS working on apq8064, but it requires fixes in the MMCC
driver, in the MDP4 driver and in DTS. I need to clean up them first
before even attempting to send them out. Also a PWM/LPG driver would
help as otherwise the power supply is quick to be overloaded by the
backlight.

Nevertheless, LVDS/LCDC-only MDP4 is a valid usecase, but it has to be
handled in the driver rather than in the core lib.

-- 
With best wishes
Dmitry

Re: [PATCH] component: Support masters with no subcomponents

Posted by Linus Walleij 1 week, 5 days ago

On Thu, Apr 18, 2024 at 1:36 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:

> I have LVDS working on apq8064, but it requires fixes in the MMCC
> driver, in the MDP4 driver and in DTS. I need to clean up them first
> before even attempting to send them out. Also a PWM/LPG driver would
> help as otherwise the power supply is quick to be overloaded by the
> backlight.

Thanks then I bet the prototype 8060 MMCC driver needs similar fixes
before it will work as well, so we should work to merge this, then look at
8060 support after that.

Yours,
Linus Walleij

Re: [PATCH] component: Support masters with no subcomponents

Posted by Dmitry Baryshkov 1 week, 5 days ago

On Fri, Apr 19, 2024 at 10:22:19AM +0200, Linus Walleij wrote:
> On Thu, Apr 18, 2024 at 1:36 AM Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> 
> > I have LVDS working on apq8064, but it requires fixes in the MMCC
> > driver, in the MDP4 driver and in DTS. I need to clean up them first
> > before even attempting to send them out. Also a PWM/LPG driver would
> > help as otherwise the power supply is quick to be overloaded by the
> > backlight.
> 
> Thanks then I bet the prototype 8060 MMCC driver needs similar fixes
> before it will work as well, so we should work to merge this, then look at
> 8060 support after that.

Once I have time to cleanup them, I'll post 8064-related fixes.

-- 
With best wishes
Dmitry

Re: [PATCH] component: Support masters with no subcomponents

Posted by Dmitry Baryshkov 2 weeks ago

On Wed, Apr 17, 2024 at 11:12:09AM +0200, Herman van Hazendonk wrote:
> This happens in the MSM DRM driver when it is used
> without any subcomponents, which is a special corner
> case.
> 
> If the MDP4 is used with nothing but the LVDS display,
> we get this problem that no components are found since
> the LVDS is part of the MDP4 itself.
> 
> We cannot use a NULL match, so create a dummy match
> with no components for this case so the driver will
> still probe nicely without adding a secondary
> complicated probe() path to the MSM DRM driver.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
> ---
> This happens in the MSM DRM driver when it is used
> without any subcomponents, which is a special corner
> case.
> 
> If the MDP4 is used with nothing but the LVDS display,
> we get this problem that no components are found since
> the LVDS is part of the MDP4 itself.
> 
> We cannot use a NULL match, so create a dummy match
> with no components for this case so the driver will
> still probe nicely without adding a secondary
> complicated probe() path to the MSM DRM driver.
> ---
>  drivers/base/component.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 741497324d78..bb79e7a77bb0 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -497,6 +497,10 @@ static void free_aggregate_device(struct aggregate_device *adev)
>  	kfree(adev);
>  }
>  
> +static struct component_match dummy_match = {
> +	.num = 0,
> +};

I think it's better to handle this in the MDP4 driver code.

Also note that LVDS / LCDC support should be fixed anyway. The DT needs
to pass LCDC clock (which it doesn't). apq8064 uses DSI2 clock after
reparenting it onto the LVDS clock generated by MDP4. Previous
generations should have a separate LCDC clock coming from MMCC.

> +
>  /**
>   * component_master_add_with_match - register an aggregate driver
>   * @parent: parent device of the aggregate driver
> @@ -516,6 +520,9 @@ int component_master_add_with_match(struct device *parent,
>  	struct aggregate_device *adev;
>  	int ret;
>  
> +	if (!match)
> +		match = &dummy_match;
> +
>  	/* Reallocate the match array for its true size */
>  	ret = component_match_realloc(match, match->num);
>  	if (ret)
> 
> ---
> base-commit: 96fca68c4fbf77a8185eb10f7557e23352732ea2
> change-id: 20240417-component-dummy-a9aae5ac7234
> 
> Best regards,
> -- 
> Herman van Hazendonk <github.com@herrie.org>
> 

-- 
With best wishes
Dmitry

Re: [PATCH] component: Support masters with no subcomponents

Posted by Greg Kroah-Hartman 2 weeks ago

On Wed, Apr 17, 2024 at 11:12:09AM +0200, Herman van Hazendonk wrote:
> This happens in the MSM DRM driver when it is used
> without any subcomponents, which is a special corner
> case.
> 
> If the MDP4 is used with nothing but the LVDS display,
> we get this problem that no components are found since
> the LVDS is part of the MDP4 itself.
> 
> We cannot use a NULL match, so create a dummy match
> with no components for this case so the driver will
> still probe nicely without adding a secondary
> complicated probe() path to the MSM DRM driver.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
> ---
> This happens in the MSM DRM driver when it is used
> without any subcomponents, which is a special corner
> case.
> 
> If the MDP4 is used with nothing but the LVDS display,
> we get this problem that no components are found since
> the LVDS is part of the MDP4 itself.
> 
> We cannot use a NULL match, so create a dummy match
> with no components for this case so the driver will
> still probe nicely without adding a secondary
> complicated probe() path to the MSM DRM driver.

Why is the text duplicated here twice?

Also, why are you adding complexity to the core for something that has
not been an issue for any other device?  Shouldn't the driver need to
handle this instead if it wishes to use the component code?  Will this
change affect any other in-tree user?

thanks,

greg k-h