linux-kernel - Re: [PATCH RFC 08/11] drm/msm/dsi: Add support for SM8750

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7255ae24-983d-452c-bd6d-85804c367f8f@linaro.org>
Date: Mon, 13 Jan 2025 12:02:54 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar
 <quic_abhinavk@...cinc.com>, Sean Paul <sean@...rly.run>,
 Marijn Suijten <marijn.suijten@...ainline.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Krishna Manikandan <quic_mkrishn@...cinc.com>,
 Jonathan Marek <jonathan@...ek.ca>, Kuogee Hsieh <quic_khsieh@...cinc.com>,
 Neil Armstrong <neil.armstrong@...aro.org>, linux-arm-msm@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Srini Kandagatla <srinivas.kandagatla@...aro.org>
Subject: Re: [PATCH RFC 08/11] drm/msm/dsi: Add support for SM8750

On 13/01/2025 09:29, Dmitry Baryshkov wrote:
> On Fri, Jan 10, 2025 at 01:43:28PM +0100, Krzysztof Kozlowski wrote:
>> On 10/01/2025 10:17, Dmitry Baryshkov wrote:
>>> On Fri, Jan 10, 2025 at 09:59:26AM +0100, Krzysztof Kozlowski wrote:
>>>> On 10/01/2025 00:18, Dmitry Baryshkov wrote:
>>>>> On Thu, Jan 09, 2025 at 02:08:35PM +0100, Krzysztof Kozlowski wrote:
>>>>>> Add support for DSI PHY v7.0 on Qualcomm SM8750 SoC which comes with two
>>>>>> differences worth noting:
>>>>>>
>>>>>> 1. ICODE_ACCUM_STATUS_LOW and ALOG_OBSV_BUS_STATUS_1 registers - their
>>>>>>    offsets were just switched.  Currently these registers are not used
>>>>>>    in the driver, so the easiest is to document both but keep them
>>>>>>    commented out to avoid conflict.
>>>>>>
>>>>>> 2. DSI PHY PLLs, the parents of pixel and byte clocks, cannot be used as
>>>>>>    parents before they are prepared and initial rate is set.  Therefore
>>>>>>    assigned-clock-parents are not working here and driver is responsible
>>>>>>    for reparenting clocks with proper procedure: see dsi_clk_init_6g_v2_9().
>>>>>
>>>>> Isn't it a description of CLK_SET_PARENT_GATE and/or
>>>>
>>>> No - must be gated accross reparent - so opposite.
>>>>
>>>>> CLK_OPS_PARENT_ENABLE ?
>>>>
>>>> Yes, but does not work. Probably enabling parent, before
>>>> assigned-clocks-parents, happens still too early:
>>>>
>>>> [    1.623554] DSI PLL(0) lock failed, status=0x00000000
>>>> [    1.623556] PLL(0) lock failed
>>>> [    1.624650] ------------[ cut here ]------------
>>>> [    1.624651] disp_cc_mdss_byte0_clk_src: rcg didn't update its
>>>> configuration.
>>>>
>>>> Or maybe something is missing in the DSI PHY PLL driver?
>>>
>>> Do you have the no-zero-freq workaround?
>>
>> Yes, it is necessary also for my variant. I did not include it here, but
>> I should mention it in the cover letter.
> 
> Could you please possibly backtrace the corresponding enable() calls?


It's the same backtrace I shared some time ago in internal discussions:
https://pastebin.com/kxUFgzD9
Unless you ask for some other backtrace?

> I'd let Stephen and/or Bjorn or Konrad to correct me, but I think that
> such requirement should be handled by the framework instead of having
> the drivers to manually reparent the clocks.

I don't know how exactly you would like to solve it. The clocks can be
reparented only after some other device specific enable sequence. It's
the third device here, but not reflected in the clocks hierarchy. Maybe
it's the result how entire Display device nodes were designed in the
first place?

Assigned clocks are between DSI PHY and DISP cc, but they are a property
of DSI controller. This looks exactly too specific for core to handle
and drivers, not framework, should manually reparent such clocks.
Otherwise we need
"clk_pre_prepare_callback_if_we_are_called_when_phy_is_disabled" sort of
callback.





Best regards,
Krzysztof