diff mbox series

[v2,1/1] arm64: defconfig: Enable SDM660 Clock Controllers

Message ID 20231115205318.2536441-1-pvorel@suse.cz
State New
Headers show
Series [v2,1/1] arm64: defconfig: Enable SDM660 Clock Controllers | expand

Commit Message

Petr Vorel Nov. 15, 2023, 8:53 p.m. UTC 
From: Petr Vorel <petr.vorel@gmail.com>

Enable support for the multimedia clock controller on SDM660 devices
and graphics clock controller on SDM630/636/660 devices.

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
Changes v1->v2:
* added commit message (not just the subject)

NOTE motivation for this is that some not yet mainlined DTS already use
both:

https://github.com/sdm660-mainline/linux/blob/sdm660-next-stable/arch/arm64/boot/dts/qcom/sdm636-asus-x00td.dts

Kind regards,
Petr

 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

Comments

Konrad Dybcio Dec. 7, 2023, 7:26 p.m. UTC | #1 
On 12/7/23 19:54, Dmitry Baryshkov wrote:
> On Thu, 7 Dec 2023 at 18:27, Bjorn Andersson <andersson@kernel.org> wrote:
>>
>> On Wed, Nov 15, 2023 at 09:53:18PM +0100, Petr Vorel wrote:
>>> From: Petr Vorel <petr.vorel@gmail.com>
>>>
>>> Enable support for the multimedia clock controller on SDM660 devices
>>> and graphics clock controller on SDM630/636/660 devices.
>>>
>>> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
>>> ---
>>> Changes v1->v2:
>>> * added commit message (not just the subject)
>>>
>>> NOTE motivation for this is that some not yet mainlined DTS already use
>>> both:
>>>
>>> https://github.com/sdm660-mainline/linux/blob/sdm660-next-stable/arch/arm64/boot/dts/qcom/sdm636-asus-x00td.dts
>>>
>>> Kind regards,
>>> Petr
>>>
>>>   arch/arm64/configs/defconfig | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>>> index acba803835b9..10a098aa8b1b 100644
>>> --- a/arch/arm64/configs/defconfig
>>> +++ b/arch/arm64/configs/defconfig
>>> @@ -1235,6 +1235,8 @@ CONFIG_SC_GCC_8180X=y
>>>   CONFIG_SC_GCC_8280XP=y
>>>   CONFIG_SC_GPUCC_8280XP=m
>>>   CONFIG_SC_LPASSCC_8280XP=m
>>> +CONFIG_SDM_MMCC_660=m
>>> +CONFIG_SDM_GPUCC_660=y
>>
>> I'd expect the GPU clock controller to be a module, can you please
>> clarify why it needs to be builtin?
> 
> To allow the display to be enabled early enough?
That sounds like a terrible bug in drm/msm.. Display should
be wholly separate from Adreno.

Konrad
Dmitry Baryshkov Dec. 7, 2023, 8:50 p.m. UTC | #2 
On Thu, 7 Dec 2023 at 21:26, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 12/7/23 19:54, Dmitry Baryshkov wrote:
> > On Thu, 7 Dec 2023 at 18:27, Bjorn Andersson <andersson@kernel.org> wrote:
> >>
> >> On Wed, Nov 15, 2023 at 09:53:18PM +0100, Petr Vorel wrote:
> >>> From: Petr Vorel <petr.vorel@gmail.com>
> >>>
> >>> Enable support for the multimedia clock controller on SDM660 devices
> >>> and graphics clock controller on SDM630/636/660 devices.
> >>>
> >>> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> >>> ---
> >>> Changes v1->v2:
> >>> * added commit message (not just the subject)
> >>>
> >>> NOTE motivation for this is that some not yet mainlined DTS already use
> >>> both:
> >>>
> >>> https://github.com/sdm660-mainline/linux/blob/sdm660-next-stable/arch/arm64/boot/dts/qcom/sdm636-asus-x00td.dts
> >>>
> >>> Kind regards,
> >>> Petr
> >>>
> >>>   arch/arm64/configs/defconfig | 2 ++
> >>>   1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> >>> index acba803835b9..10a098aa8b1b 100644
> >>> --- a/arch/arm64/configs/defconfig
> >>> +++ b/arch/arm64/configs/defconfig
> >>> @@ -1235,6 +1235,8 @@ CONFIG_SC_GCC_8180X=y
> >>>   CONFIG_SC_GCC_8280XP=y
> >>>   CONFIG_SC_GPUCC_8280XP=m
> >>>   CONFIG_SC_LPASSCC_8280XP=m
> >>> +CONFIG_SDM_MMCC_660=m
> >>> +CONFIG_SDM_GPUCC_660=y
> >>
> >> I'd expect the GPU clock controller to be a module, can you please
> >> clarify why it needs to be builtin?
> >
> > To allow the display to be enabled early enough?
> That sounds like a terrible bug in drm/msm.. Display should
> be wholly separate from Adreno.

Let me quote Rob's email ([1])

Userspace does have better support for split display/gpu these days
than it did when drm/msm was first merged.  It _might_ just work if
one device only advertised DRIVER_RENDER and the other
MODESET/ATOMIC.. but I'd be a bit concerned about breaking things.  I
guess you could try some sort of kconfig knob to have two "msm"
devices and see what breaks, but I'm a bit skeptical that we could
make this the default anytime soon.

[1] https://lore.kernel.org/dri-devel/CAF6AEGs89FRmFsENLkP-Dg1ZJN2LzCfxY2-+do9jH9b8L-XZxg@mail.gmail.com/



--
With best wishes
Dmitry
Petr Vorel Dec. 7, 2023, 9:48 p.m. UTC | #3 
Hi all,

[ Cc Alexey Minnekhanov ]

> On Thu, 7 Dec 2023 at 21:26, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:

> > On 12/7/23 19:54, Dmitry Baryshkov wrote:
> > > On Thu, 7 Dec 2023 at 18:27, Bjorn Andersson <andersson@kernel.org> wrote:

> > >> On Wed, Nov 15, 2023 at 09:53:18PM +0100, Petr Vorel wrote:
> > >>> From: Petr Vorel <petr.vorel@gmail.com>

> > >>> Enable support for the multimedia clock controller on SDM660 devices
> > >>> and graphics clock controller on SDM630/636/660 devices.

> > >>> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > >>> ---
> > >>> Changes v1->v2:
> > >>> * added commit message (not just the subject)

> > >>> NOTE motivation for this is that some not yet mainlined DTS already use
> > >>> both:

> > >>> https://github.com/sdm660-mainline/linux/blob/sdm660-next-stable/arch/arm64/boot/dts/qcom/sdm636-asus-x00td.dts

> > >>> Kind regards,
> > >>> Petr

> > >>>   arch/arm64/configs/defconfig | 2 ++
> > >>>   1 file changed, 2 insertions(+)

> > >>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > >>> index acba803835b9..10a098aa8b1b 100644
> > >>> --- a/arch/arm64/configs/defconfig
> > >>> +++ b/arch/arm64/configs/defconfig
> > >>> @@ -1235,6 +1235,8 @@ CONFIG_SC_GCC_8180X=y
> > >>>   CONFIG_SC_GCC_8280XP=y
> > >>>   CONFIG_SC_GPUCC_8280XP=m
> > >>>   CONFIG_SC_LPASSCC_8280XP=m
> > >>> +CONFIG_SDM_MMCC_660=m
> > >>> +CONFIG_SDM_GPUCC_660=y

> > >> I'd expect the GPU clock controller to be a module, can you please
> > >> clarify why it needs to be builtin?

> > > To allow the display to be enabled early enough?

Yes, I feared that it would not work when it's a module.
Also, we already have CONFIG_SDM_GPUCC_845=y.
I suppose I'm wrong, but I don't have any sdm660 device to test that.

BTW people who are using this use both as builtin (CONFIG_SDM_MMCC_660) [2], but
maybe it's just to help testing (boot the kernel and don't bother with modules).

@Alexey, you added sdm660_defconfig [2], do you have sdm660 based device to test
if both options work well when compiled as modules?

> > That sounds like a terrible bug in drm/msm.. Display should
> > be wholly separate from Adreno.

> Let me quote Rob's email ([1])

> Userspace does have better support for split display/gpu these days
> than it did when drm/msm was first merged.  It _might_ just work if
> one device only advertised DRIVER_RENDER and the other
> MODESET/ATOMIC.. but I'd be a bit concerned about breaking things.  I
> guess you could try some sort of kconfig knob to have two "msm"
> devices and see what breaks, but I'm a bit skeptical that we could
> make this the default anytime soon.

Thanks for pointing out this.

Kind regards,
Petr

> [1] https://lore.kernel.org/dri-devel/CAF6AEGs89FRmFsENLkP-Dg1ZJN2LzCfxY2-+do9jH9b8L-XZxg@mail.gmail.com/
[2] https://github.com/sdm660-mainline/linux/blob/sdm660-next-stable/arch/arm64/configs/sdm660_defconfig#L504-L505
Bjorn Andersson Dec. 9, 2023, 3:08 a.m. UTC | #4 
On Thu, Dec 07, 2023 at 08:54:32PM +0200, Dmitry Baryshkov wrote:
> On Thu, 7 Dec 2023 at 18:27, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Wed, Nov 15, 2023 at 09:53:18PM +0100, Petr Vorel wrote:
> > > From: Petr Vorel <petr.vorel@gmail.com>
> > >
> > > Enable support for the multimedia clock controller on SDM660 devices
> > > and graphics clock controller on SDM630/636/660 devices.
> > >
> > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > > ---
> > > Changes v1->v2:
> > > * added commit message (not just the subject)
> > >
> > > NOTE motivation for this is that some not yet mainlined DTS already use
> > > both:
> > >
> > > https://github.com/sdm660-mainline/linux/blob/sdm660-next-stable/arch/arm64/boot/dts/qcom/sdm636-asus-x00td.dts
> > >
> > > Kind regards,
> > > Petr
> > >
> > >  arch/arm64/configs/defconfig | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > > index acba803835b9..10a098aa8b1b 100644
> > > --- a/arch/arm64/configs/defconfig
> > > +++ b/arch/arm64/configs/defconfig
> > > @@ -1235,6 +1235,8 @@ CONFIG_SC_GCC_8180X=y
> > >  CONFIG_SC_GCC_8280XP=y
> > >  CONFIG_SC_GPUCC_8280XP=m
> > >  CONFIG_SC_LPASSCC_8280XP=m
> > > +CONFIG_SDM_MMCC_660=m
> > > +CONFIG_SDM_GPUCC_660=y
> >
> > I'd expect the GPU clock controller to be a module, can you please
> > clarify why it needs to be builtin?
> 
> To allow the display to be enabled early enough?
> 

If that's your goal, then it might be less optimal to have MMCC as a
module...

We should keep drivers essential for reaching the ramdisk as builtin
(which pretty much means the stuff necessary to establish /dev/console),
and then the rest as modules.

There are several here which are =y because it used to be that probe
deferral on power-domains didn't work. We should drop those to =m as
well...

Thanks,
Bjorn

> >
> > Regards,
> > Bjorn
> >
> > >  CONFIG_SDM_CAMCC_845=m
> > >  CONFIG_SDM_GPUCC_845=y
> > >  CONFIG_SDM_VIDEOCC_845=y
> > > --
> > > 2.42.0
> > >
> >
> 
> 
> -- 
> With best wishes
> Dmitry
diff mbox series

Patch

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index acba803835b9..10a098aa8b1b 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1235,6 +1235,8 @@  CONFIG_SC_GCC_8180X=y
 CONFIG_SC_GCC_8280XP=y
 CONFIG_SC_GPUCC_8280XP=m
 CONFIG_SC_LPASSCC_8280XP=m
+CONFIG_SDM_MMCC_660=m
+CONFIG_SDM_GPUCC_660=y
 CONFIG_SDM_CAMCC_845=m
 CONFIG_SDM_GPUCC_845=y
 CONFIG_SDM_VIDEOCC_845=y