Re: [PATCH] component: Support masters with no subcomponents
On Fri, Apr 19, 2024 at 10:22:19AM +0200, Linus Walleij wrote: > On Thu, Apr 18, 2024 at 1:36 AM Dmitry Baryshkov > 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
On Thu, Apr 18, 2024 at 1:36 AM Dmitry Baryshkov 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
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
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
[PATCH] component: Support masters with no subcomponents
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 Signed-off-by: Herman van Hazendonk --- 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
Re: [PATCH] component: Support masters with no subcomponents
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 > Signed-off-by: Herman van Hazendonk > --- > 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 > -- With best wishes Dmitry
Re: [PATCH] component: Support masters with no subcomponents
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 > Signed-off-by: Herman van Hazendonk > --- > 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