[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <858be1b7-0183-47b3-97b5-7d162b5748d3@quicinc.com>
Date: Sat, 3 May 2025 12:59:21 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
CC: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Jessica Zhang <jesszhan@...cinc.com>,
Abhinav Kumar <abhinavk@...cinc.com>, Abel Vesa <abel.vesa@...aro.org>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC/WIP 1/4] arm64: dts: qcom: sm8750: Add display (MDSS)
with Display CC
On 5/2/2025 10:51 PM, Dmitry Baryshkov wrote:
> On Tue, Apr 29, 2025 at 04:07:24PM -0700, Abhinav Kumar wrote:
>>
>>
>> On 4/28/2025 2:31 PM, Konrad Dybcio wrote:
>>> On 4/24/25 3:04 PM, Krzysztof Kozlowski wrote:
>>>> Add device nodes for entire display: MDSS, DPU, DSI, DSI PHYs,
>>>> DisplayPort and Display Clock Controller.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>>>
>>>> ---
>>>
>>> [...]
>>>
>>>> + mdp_opp_table: opp-table {
>>>> + compatible = "operating-points-v2";
>>>> +
>>>
>>> The computer tells me there's also a 156 MHz rate @ SVS_D1
>>>
>>> Maybe Abhinav could chime in whether we should add it or not
>>>
>>
>> Yes I also see a 156Mhz for LOW_SVS_D1 but we had a similar entry even for
>> sm8650 and did not publish it in the dt.
>>
>> It was present till sm8450.dtsi but dropped in sm8550/sm8650 even though
>> LOW_SVS_D1 is present even on those.
>>
>> I think the reason could be that the displays being used on the reference
>> boards will need a pixel clock of atleast >= low_svs and the MDP clock
>> usually depends on the value of the DSI pixel clock (which has a fixed
>> relationship to the byte clock) to maintain the data rate. So as a result
>> perhaps even if we add it, for most displays this level will be unused.
>>
>> If we end up using displays which are so small that the pixel clock
>> requirement will be even lower than low_svs, we can add those.
>>
>> OR as an alternative, we can leave this patch as it is and add the
>> low_svs_d1 for all chipsets which support it together in another series that
>> way it will have the full context of why we are adding it otherwise it will
>> look odd again of why sm8550/sm8650 was left out but added in sm8750.
>
> I think it's better to describe hardware accurately, even if the
> particular entry ends up being unused. I'd vote for this option.
>
>>> [...]
>>>
>>>> + mdss_dsi_opp_table: opp-table {
>>>> + compatible = "operating-points-v2";
>>>> +
>>>
>>> Similarly there's a 140.63 MHz rate at SVS_D1, but it seems odd
>>> with the decimals
>>
>> For this one, yes its true that LOW_SVS_D1 is 140.63Mhz for sm8750 but this
>> voltage corner was somehow never used for DSI byte clock again I am thinking
>> this is because for the display resolutions we use, we will always be >=
>> low_svs so the low_svs_d1 will never hit even if we add it.
>
> Please add all voltage/frequency corners. Think about low-res DP or
> low-res, low-rate WB.
>
Sounds good, lets go ahead and add all the voltage/freq corners.
Like I noted, even for sm8550/sm8650 the low_svs_d1 was missed out, so
if we are adding it for sm8750 now in this series, a follow up patch
should also be sent to add them for sm8550/sm8650 as well. That way we
will fix them all up together and this does not come across as a
discrepancy.
Powered by blists - more mailing lists