diff mbox series

nvmem: mtk-efuse: Enable GPU speed bin post-processing for MT8188

Message ID 20241223100648.2166754-1-wenst@chromium.org (mailing list archive)
State New
Headers show
Series nvmem: mtk-efuse: Enable GPU speed bin post-processing for MT8188 | expand

Commit Message

Chen-Yu Tsai Dec. 23, 2024, 10:06 a.m. UTC
Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
The data needs post-processing into a format that the OPP framework can
use.

Add a compatible match for MT8188 efuse with post-processing enabled.

Cc: <stable@vger.kernel.org>
Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---

I'm not exactly sure about pointing to the dt bindings commit for the
fixes tag.
---
 drivers/nvmem/mtk-efuse.c | 1 +
 1 file changed, 1 insertion(+)

Comments

AngeloGioacchino Del Regno Dec. 23, 2024, 11:11 a.m. UTC | #1
Il 23/12/24 11:06, Chen-Yu Tsai ha scritto:
> Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
> The data needs post-processing into a format that the OPP framework can
> use.
> 
> Add a compatible match for MT8188 efuse with post-processing enabled.
> 

Let's just change the MT8188 compatible list to

compatible = "mediatek,mt8188-efuse", "mediatek,mt8186-efuse";

instead :-)

Cheers,
Angelo

> Cc: <stable@vger.kernel.org>
> Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> 
> I'm not exactly sure about pointing to the dt bindings commit for the
> fixes tag.
> ---
>   drivers/nvmem/mtk-efuse.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> index af953e1d9230..e8409e1e7fac 100644
> --- a/drivers/nvmem/mtk-efuse.c
> +++ b/drivers/nvmem/mtk-efuse.c
> @@ -112,6 +112,7 @@ static const struct mtk_efuse_pdata mtk_efuse_pdata = {
>   static const struct of_device_id mtk_efuse_of_match[] = {
>   	{ .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
>   	{ .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
> +	{ .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
>   	{ .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
>   	{/* sentinel */},
>   };
Chen-Yu Tsai Dec. 23, 2024, 11:24 a.m. UTC | #2
On Mon, Dec 23, 2024 at 7:11 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 23/12/24 11:06, Chen-Yu Tsai ha scritto:
> > Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
> > The data needs post-processing into a format that the OPP framework can
> > use.
> >
> > Add a compatible match for MT8188 efuse with post-processing enabled.
> >
>
> Let's just change the MT8188 compatible list to
>
> compatible = "mediatek,mt8188-efuse", "mediatek,mt8186-efuse";

That would be "mediatek,mt8188-efuse", "mediatek,mt8186-efuse", "mediatek,efuse"
then?

Fine by me. :D

ChenYu

> instead :-)
>
> Cheers,
> Angelo
>
> > Cc: <stable@vger.kernel.org>
> > Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >
> > I'm not exactly sure about pointing to the dt bindings commit for the
> > fixes tag.
> > ---
> >   drivers/nvmem/mtk-efuse.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> > index af953e1d9230..e8409e1e7fac 100644
> > --- a/drivers/nvmem/mtk-efuse.c
> > +++ b/drivers/nvmem/mtk-efuse.c
> > @@ -112,6 +112,7 @@ static const struct mtk_efuse_pdata mtk_efuse_pdata = {
> >   static const struct of_device_id mtk_efuse_of_match[] = {
> >       { .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
> >       { .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
> > +     { .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
> >       { .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
> >       {/* sentinel */},
> >   };
>
AngeloGioacchino Del Regno Dec. 23, 2024, 11:43 a.m. UTC | #3
Il 23/12/24 12:24, Chen-Yu Tsai ha scritto:
> On Mon, Dec 23, 2024 at 7:11 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 23/12/24 11:06, Chen-Yu Tsai ha scritto:
>>> Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
>>> The data needs post-processing into a format that the OPP framework can
>>> use.
>>>
>>> Add a compatible match for MT8188 efuse with post-processing enabled.
>>>
>>
>> Let's just change the MT8188 compatible list to
>>
>> compatible = "mediatek,mt8188-efuse", "mediatek,mt8186-efuse";
> 
> That would be "mediatek,mt8188-efuse", "mediatek,mt8186-efuse", "mediatek,efuse"
> then?
> 

No, we're dropping the generic "mediatek,efuse".

Cheers!

> Fine by me. :D
> 
> ChenYu
> 
>> instead :-)
>>
>> Cheers,
>> Angelo
>>
>>> Cc: <stable@vger.kernel.org>
>>> Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>> ---
>>>
>>> I'm not exactly sure about pointing to the dt bindings commit for the
>>> fixes tag.
>>> ---
>>>    drivers/nvmem/mtk-efuse.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
>>> index af953e1d9230..e8409e1e7fac 100644
>>> --- a/drivers/nvmem/mtk-efuse.c
>>> +++ b/drivers/nvmem/mtk-efuse.c
>>> @@ -112,6 +112,7 @@ static const struct mtk_efuse_pdata mtk_efuse_pdata = {
>>>    static const struct of_device_id mtk_efuse_of_match[] = {
>>>        { .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
>>>        { .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
>>> +     { .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
>>>        { .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
>>>        {/* sentinel */},
>>>    };
>>
Chen-Yu Tsai Dec. 23, 2024, 3:57 p.m. UTC | #4
On Mon, Dec 23, 2024 at 7:43 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 23/12/24 12:24, Chen-Yu Tsai ha scritto:
> > On Mon, Dec 23, 2024 at 7:11 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 23/12/24 11:06, Chen-Yu Tsai ha scritto:
> >>> Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
> >>> The data needs post-processing into a format that the OPP framework can
> >>> use.
> >>>
> >>> Add a compatible match for MT8188 efuse with post-processing enabled.
> >>>
> >>
> >> Let's just change the MT8188 compatible list to
> >>
> >> compatible = "mediatek,mt8188-efuse", "mediatek,mt8186-efuse";
> >
> > That would be "mediatek,mt8188-efuse", "mediatek,mt8186-efuse", "mediatek,efuse"
> > then?
> >
>
> No, we're dropping the generic "mediatek,efuse".

That means we also drop it for MT8186?

Thinking about it more, I think it's stretching things a bit. The hardware
is clearly backwards compatible, or we wouldn't even be reading values
out correctly. The only difference now with MT8186 and MT8188 is that
they have a speed-bin field with a value that we want passed to the OPP
framework, and the interpretation of that value is not really part of
the efuse's hardware. We chose to do the conversion in the efuse driver,
but we could also have done it in the GPU driver.

What I'm saying is that we should not need to change the compatible strings
to make this work.


ChenYu

> Cheers!
>
> > Fine by me. :D
> >
> > ChenYu
> >
> >> instead :-)
> >>
> >> Cheers,
> >> Angelo
> >>
> >>> Cc: <stable@vger.kernel.org>
> >>> Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
> >>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> >>> ---
> >>>
> >>> I'm not exactly sure about pointing to the dt bindings commit for the
> >>> fixes tag.
> >>> ---
> >>>    drivers/nvmem/mtk-efuse.c | 1 +
> >>>    1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> >>> index af953e1d9230..e8409e1e7fac 100644
> >>> --- a/drivers/nvmem/mtk-efuse.c
> >>> +++ b/drivers/nvmem/mtk-efuse.c
> >>> @@ -112,6 +112,7 @@ static const struct mtk_efuse_pdata mtk_efuse_pdata = {
> >>>    static const struct of_device_id mtk_efuse_of_match[] = {
> >>>        { .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
> >>>        { .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
> >>> +     { .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
> >>>        { .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
> >>>        {/* sentinel */},
> >>>    };
> >>
>
>
AngeloGioacchino Del Regno Dec. 23, 2024, 4:08 p.m. UTC | #5
Il 23/12/24 16:57, Chen-Yu Tsai ha scritto:
> On Mon, Dec 23, 2024 at 7:43 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 23/12/24 12:24, Chen-Yu Tsai ha scritto:
>>> On Mon, Dec 23, 2024 at 7:11 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com> wrote:
>>>>
>>>> Il 23/12/24 11:06, Chen-Yu Tsai ha scritto:
>>>>> Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
>>>>> The data needs post-processing into a format that the OPP framework can
>>>>> use.
>>>>>
>>>>> Add a compatible match for MT8188 efuse with post-processing enabled.
>>>>>
>>>>
>>>> Let's just change the MT8188 compatible list to
>>>>
>>>> compatible = "mediatek,mt8188-efuse", "mediatek,mt8186-efuse";
>>>
>>> That would be "mediatek,mt8188-efuse", "mediatek,mt8186-efuse", "mediatek,efuse"
>>> then?
>>>
>>
>> No, we're dropping the generic "mediatek,efuse".
> 
> That means we also drop it for MT8186?
> 
> Thinking about it more, I think it's stretching things a bit. The hardware
> is clearly backwards compatible, or we wouldn't even be reading values
> out correctly. The only difference now with MT8186 and MT8188 is that
> they have a speed-bin field with a value that we want passed to the OPP
> framework, and the interpretation of that value is not really part of
> the efuse's hardware. We chose to do the conversion in the efuse driver,
> but we could also have done it in the GPU driver.
> 
> What I'm saying is that we should not need to change the compatible strings
> to make this work.
> 

No we don't forcefully have to drop it from MT8186, and doing so would be kind
of hard and actually producing unnecessary breakages with (very) old kernels.

Just add a `deprecated: true` to the binding that wants `mediatek,efuse` and
start with MT8188, where 8188 is in enum and 8186 is const.

We can do MT8188 because that'll still work even with old kernels (since MT8186
is there since before MT8188 was introduced), and it's something to enable a new
feature.
This means that there's not going to be any breakage with new DT and old kernel.

I want the mediatek,efuse binding to be like the majority of the others across
the kernel, so, no generic compatible.

Cheers,
Angelo

> 
> ChenYu
> 
>> Cheers!
>>
>>> Fine by me. :D
>>>
>>> ChenYu
>>>
>>>> instead :-)
>>>>
>>>> Cheers,
>>>> Angelo
>>>>
>>>>> Cc: <stable@vger.kernel.org>
>>>>> Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
>>>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>>>> ---
>>>>>
>>>>> I'm not exactly sure about pointing to the dt bindings commit for the
>>>>> fixes tag.
>>>>> ---
>>>>>     drivers/nvmem/mtk-efuse.c | 1 +
>>>>>     1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
>>>>> index af953e1d9230..e8409e1e7fac 100644
>>>>> --- a/drivers/nvmem/mtk-efuse.c
>>>>> +++ b/drivers/nvmem/mtk-efuse.c
>>>>> @@ -112,6 +112,7 @@ static const struct mtk_efuse_pdata mtk_efuse_pdata = {
>>>>>     static const struct of_device_id mtk_efuse_of_match[] = {
>>>>>         { .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
>>>>>         { .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
>>>>> +     { .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
>>>>>         { .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
>>>>>         {/* sentinel */},
>>>>>     };
>>>>
>>
Chen-Yu Tsai Dec. 23, 2024, 4:15 p.m. UTC | #6
On Tue, Dec 24, 2024 at 12:08 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 23/12/24 16:57, Chen-Yu Tsai ha scritto:
> > On Mon, Dec 23, 2024 at 7:43 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 23/12/24 12:24, Chen-Yu Tsai ha scritto:
> >>> On Mon, Dec 23, 2024 at 7:11 PM AngeloGioacchino Del Regno
> >>> <angelogioacchino.delregno@collabora.com> wrote:
> >>>>
> >>>> Il 23/12/24 11:06, Chen-Yu Tsai ha scritto:
> >>>>> Like the MT8186, the MT8188 stores GPU speed binning data in its efuse.
> >>>>> The data needs post-processing into a format that the OPP framework can
> >>>>> use.
> >>>>>
> >>>>> Add a compatible match for MT8188 efuse with post-processing enabled.
> >>>>>
> >>>>
> >>>> Let's just change the MT8188 compatible list to
> >>>>
> >>>> compatible = "mediatek,mt8188-efuse", "mediatek,mt8186-efuse";
> >>>
> >>> That would be "mediatek,mt8188-efuse", "mediatek,mt8186-efuse", "mediatek,efuse"
> >>> then?
> >>>
> >>
> >> No, we're dropping the generic "mediatek,efuse".
> >
> > That means we also drop it for MT8186?
> >
> > Thinking about it more, I think it's stretching things a bit. The hardware
> > is clearly backwards compatible, or we wouldn't even be reading values
> > out correctly. The only difference now with MT8186 and MT8188 is that
> > they have a speed-bin field with a value that we want passed to the OPP
> > framework, and the interpretation of that value is not really part of
> > the efuse's hardware. We chose to do the conversion in the efuse driver,
> > but we could also have done it in the GPU driver.
> >
> > What I'm saying is that we should not need to change the compatible strings
> > to make this work.
> >
>
> No we don't forcefully have to drop it from MT8186, and doing so would be kind
> of hard and actually producing unnecessary breakages with (very) old kernels.
>
> Just add a `deprecated: true` to the binding that wants `mediatek,efuse` and
> start with MT8188, where 8188 is in enum and 8186 is const.
>
> We can do MT8188 because that'll still work even with old kernels (since MT8186
> is there since before MT8188 was introduced), and it's something to enable a new
> feature.
> This means that there's not going to be any breakage with new DT and old kernel.
>
> I want the mediatek,efuse binding to be like the majority of the others across
> the kernel, so, no generic compatible.

In that case shouldn't the fallback be the oldest SoC in the list?
Maybe MT8173, which is currently marked as deprecated, so we undeprecate it?

Certainly not the MT8186.

> Cheers,
> Angelo
>
> >
> > ChenYu
> >
> >> Cheers!
> >>
> >>> Fine by me. :D
> >>>
> >>> ChenYu
> >>>
> >>>> instead :-)
> >>>>
> >>>> Cheers,
> >>>> Angelo
> >>>>
> >>>>> Cc: <stable@vger.kernel.org>
> >>>>> Fixes: ff1df1886f43 ("dt-bindings: nvmem: mediatek: efuse: Add support for MT8188")
> >>>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> >>>>> ---
> >>>>>
> >>>>> I'm not exactly sure about pointing to the dt bindings commit for the
> >>>>> fixes tag.
> >>>>> ---
> >>>>>     drivers/nvmem/mtk-efuse.c | 1 +
> >>>>>     1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> >>>>> index af953e1d9230..e8409e1e7fac 100644
> >>>>> --- a/drivers/nvmem/mtk-efuse.c
> >>>>> +++ b/drivers/nvmem/mtk-efuse.c
> >>>>> @@ -112,6 +112,7 @@ static const struct mtk_efuse_pdata mtk_efuse_pdata = {
> >>>>>     static const struct of_device_id mtk_efuse_of_match[] = {
> >>>>>         { .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
> >>>>>         { .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
> >>>>> +     { .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
> >>>>>         { .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
> >>>>>         {/* sentinel */},
> >>>>>     };
> >>>>
> >>
diff mbox series

Patch

diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
index af953e1d9230..e8409e1e7fac 100644
--- a/drivers/nvmem/mtk-efuse.c
+++ b/drivers/nvmem/mtk-efuse.c
@@ -112,6 +112,7 @@  static const struct mtk_efuse_pdata mtk_efuse_pdata = {
 static const struct of_device_id mtk_efuse_of_match[] = {
 	{ .compatible = "mediatek,mt8173-efuse", .data = &mtk_efuse_pdata },
 	{ .compatible = "mediatek,mt8186-efuse", .data = &mtk_mt8186_efuse_pdata },
+	{ .compatible = "mediatek,mt8188-efuse", .data = &mtk_mt8186_efuse_pdata },
 	{ .compatible = "mediatek,efuse", .data = &mtk_efuse_pdata },
 	{/* sentinel */},
 };