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

[Petr Vorel <pvorel@suse.cz>]

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