| 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
| ||
|
Message-ID: <26adc983-5dbf-b53f-82a5-96ecc3cd41f2@collabora.com>
Date: Fri, 8 Jul 2022 17:37:45 +0200
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: "allen-kh.cheng" <allen-kh.cheng@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>
Cc: Project_Global_Chrome_Upstream_Group@...iatek.com,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
Xiandong Wang <xiandong.wang@...iatek.com>
Subject: Re: [PATCH 1/2] dt-bindings: soc: mediatek: add mdp3 mutex support
for mt8186
Il 08/07/22 13:58, allen-kh.cheng ha scritto:
> Hi Angelo,
>
> On Fri, 2022-07-08 at 10:28 +0200, AngeloGioacchino Del Regno wrote:
>> Il 08/07/22 10:19, Matthias Brugger ha scritto:
>>>
>>>
>>> On 08/07/2022 10:14, allen-kh.cheng wrote:
>>>> Hi Angelo,
>>>>
>>>> On Thu, 2022-07-07 at 12:59 +0200, AngeloGioacchino Del Regno
>>>> wrote:
>>>>> Il 07/07/22 12:41, Matthias Brugger ha scritto:
>>>>>>
>>>>>>
>>>>>> On 07/07/2022 10:52, AngeloGioacchino Del Regno wrote:
>>>>>>> Il 05/07/22 14:26, Allen-KH Cheng ha scritto:
>>>>>>>> Add mdp3 mutex compatible for mt8186 SoC.
>>>>>>>>
>>>>>>>> Signed-off-by: Allen-KH Cheng <
>>>>>>>> allen-kh.cheng@...iatek.com>
>>>>>>>> Signed-off-by: Xiandong Wang <xiandong.wang@...iatek.com>
>>>>>>>
>>>>>>>
>>>>>>> Please drop this commit. Adding a mdp3-mutex compatible is
>>>>>>> not
>>>>>>> needed here.
>>>>>>>
>>>>>>
>>>>>> Thanks for checking. We probably would need a fallback
>>>>>> compatible.
>>>>>> We can only know
>>>>>> from the HW engineers that can confirm if the IP block is the
>>>>>> same
>>>>>> as the disp
>>>>>> mutex or a different one.
>>>>>>
>>>>>> I'll drop both patches for now until things got clear.
>>>>>>
>>>>>
>>>>> They're located in a different iospace from each other, but
>>>>> either
>>>>> the platform
>>>>> data needs to *not be* joined together, or if they're together,
>>>>> I
>>>>> would not like
>>>>> having two different compatible strings for essentially the
>>>>> same
>>>>> thing.
>>>>>
>>>>> I would at this point prefer dropping '-disp' from
>>>>> 'mediatek,mt8186-
>>>>> disp-mutex'
>>>>> so that we would be able to declare two 'mediatek,mt8186-mutex'
>>>>> in
>>>>> devicetree...
>>>>> ...or simply have two mediatek,mt8186-disp-mutex (although
>>>>> logically
>>>>> incorrect?).
>>>>>
>>>>> Cheers,
>>>>> Angelo
>>>>>
>>>>
>>>> Thanks for your opinion.
>>>>
>>>> They are two different hardwares for different address spaces.
>>>>
>>>> I think we drop '-disp' from 'mediatek,mt8186-disp-mutex' will be
>>>> excessive because we also need to modify mutex node in all exited
>>>> dts
>>>> files.
>>>>
>>>> I prefer havingg two mediatek,mt8186-disp-mutex.
>>>>
>>>> ex:
>>>> mutex: mutex@...01000 {
>>>> compatible = "mediatek,mt8186-disp-mutex";
>>>> ..
>>>> }
>>>>
>>>> mdp3_mutex0: mutex@...01000 {
>>>> compatible = "mediatek,mt8186-disp-mutex";
>>>> ...
>>>> }
>>>>
>>>> What do you think?
>>>
>>> I think that's an acceptable solution.
>>>
>>
>> I'm a bit undecided instead, now... because from what I understand
>> now,
>> the platform data fields
>>
>> .mutex_mod and .mutex_sof
>>
>> are *not valid* for mutex at 0x1b001000 but only for the instance at
>> 0x14001000.
>>
>> If we go this way, at this point, we would be free (and allowed by
>> the driver)
>> to try to set these for 0x1b001000, and to try to set MDP3 table
>> paths on
>> 0x14001000, which is something that shouldn't be logically allowed,
>> as the
>> hardware does *not* support that.
>>
>> Unless I got that wrong, and these fields for MUTEX_MOD_DISP_xxxx do
>> exist in
>> the mutex instance at 0xb001000, in which case, I fully agree with
>> Matthias.
>>
>> But otherwise, I have my doubts.
>>
>> Cheers,
>> Angelo
>>
>
> I got your point.
>
> The disp and mdp3 drivers work with the same data field beacase
> 14001000 (disp mutex) would not use .mutex_table_mod and 1b001000 (mdp3
> mutex) would not use .mutex_mod/.mutex_sof.
>
>
> How about ...
>
> static const struct mtk_mutex_data mt8186_mutex_driver_data = {
> .mutex_mod = mt8186_mutex_mod,
> .mutex_sof = mt8186_mutex_sof,
> .mutex_mod_reg = MT8183_MUTEX0_MOD0,
> .mutex_sof_reg = MT8183_MUTEX0_SOF0,
> };
>
> static const struct mtk_mutex_data mt8186_mutex_mdp_driver_data = {
> .mutex_table_mod = mt8186_mutex_table_mod,
> };
>
> { .compatible = "mediatek,mt8186-disp-mutex",
> .data = &mt8186_mutex_driver_data},
> { .compatible = "mediatek,mt8186-mdp3-mutex",
> .data = &mt8186_mutex_mdp_driver_data},
>
>
> mutex: mutex@...01000 {
> compatible = "mediatek,mt8186-disp-mutex";
> ..
> }
> mdp3_mutex0: mutex@...01000 {
> compatible = "mediatek,mt8186-mdp3-mutex";
> ...
> }
>
> Do you think that is feasible?
>
This makes a lot more sense to me.
Though, you have to also add the mod and sof regs, because the mutex instance
for MDP_MUTEX does have these registers, even though they are used for different
mods/sofs.
static const struct mtk_mutex_data mt8186_mutex_driver_data = {
.mutex_mod = mt8186_mutex_mod,
.mutex_sof = mt8186_mutex_sof,
.mutex_mod_reg = MT8183_MUTEX0_MOD0,
.mutex_sof_reg = MT8183_MUTEX0_SOF0,
};
static const struct mtk_mutex_data mt8186_mdp_mutex_driver_data = {
.mutex_mod_reg = MT8183_MUTEX0_MOD0,
.mutex_sof_reg = MT8183_MUTEX0_SOF0,
.mutex_table_mod = mt8186_mdp_mutex_table_mod,
};
P.S.: Notice that mt8186_mdp_mutex_driver_data instead of
mt8186_mutex_mdp_driver_data was chosen on purpose:
like that, we're referencing to real block names.
Regards,
Angelo
> Best Regards,
> Allen
>
>>> Regards,
>>> Matthias
>>>
>>>>
>>>> Best regards,
>>>> Allen
>>>>
>>>>>> Regards,
>>>>>> Matthias
>>>>>>
>>>>>>>> ---
>>>>>>>> .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
>>>>>>>> | 1 +
>>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git
>>>>>>>> a/Documentation/devicetree/bindings/soc/mediatek/mediatek
>>>>>>>> ,mutex
>>>>>>>> .yaml
>>>>>>>> b/Documentation/devicetree/bindings/soc/mediatek/mediatek
>>>>>>>> ,mutex
>>>>>>>> .yaml
>>>>>>>> index 627dcc3e8b32..234fa5dc07c2 100644
>>>>>>>> ---
>>>>>>>> a/Documentation/devicetree/bindings/soc/mediatek/mediatek
>>>>>>>> ,mutex
>>>>>>>> .yaml
>>>>>>>> +++
>>>>>>>> b/Documentation/devicetree/bindings/soc/mediatek/mediatek
>>>>>>>> ,mutex
>>>>>>>> .yaml
>>>>>>>> @@ -30,6 +30,7 @@ properties:
>>>>>>>> - mediatek,mt8173-disp-mutex
>>>>>>>> - mediatek,mt8183-disp-mutex
>>>>>>>> - mediatek,mt8186-disp-mutex
>>>>>>>> + - mediatek,mt8186-mdp3-mutex
>>>>>>>> - mediatek,mt8192-disp-mutex
>>>>>>>> - mediatek,mt8195-disp-mutex
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>
Powered by blists - more mailing lists