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(+)
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 */}, };
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 */}, };
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 */},
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 */},
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 */},
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 */},
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 */},
Il 23/12/24 17:15, Chen-Yu Tsai ha scritto:
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.
MT8173 doesn't have the post processing enabled.... if you can test that the code actually works for MT8173 as well, giving meaningful results, then we can just use the MT8173 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 */}, > };
On Tue, Jan 7, 2025 at 9:12 PM AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com wrote:
Il 23/12/24 17:15, Chen-Yu Tsai ha scritto:
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.
MT8173 doesn't have the post processing enabled.... if you can test that the code actually works for MT8173 as well, giving meaningful results, then we can just use the MT8173 compatible :-)
What I'm saying is (and hopefully I regenerated my thoughts correctly) that if we want to get rid of the generic compatible string, then we need a base fallback string. The MT8173 one would be the one.
The MT8186 efuse at the hardware level AFAIK is no different to the MT8173 one. They simply store bits. How the bits are allocated is defined by the layout. How the bits are interpreted is also not a property of the hardware.
In the current design the need for post-processing does not depend on the SoC itself, but rather the node name being "gpu-speedbin". Even if we turned on post processing for all models, it wouldn't matter for the other SoCs.
And the reason the conversion is in the nvmem driver (the provider) rather than panfrost (the consumer) has nothing to do with the hardware. IIRC you simply wanted to provide a generic implementation. If the conversion were in the GPU driver, we wouldn't special case MT8186 here. If the speedbin value were split into two or more cells, we probably would have stuck the conversion in the GPU driver.
So, we could have "mediatek,mt8173-efuse" replace "mediatek,efuse" as the fallback compatible, and turn on post processing for it, and things would be the same as before, and also work for MT8188. How does that sound?
Interestingly, the new "fixed layout" description actually allows for compatible strings in the NVMEM cells. That would allow matching against a cell's compatible string instead of its name.
Thanks ChenYu
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 */}, >> }; >
Il 08/01/25 07:36, Chen-Yu Tsai ha scritto:
On Tue, Jan 7, 2025 at 9:12 PM AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com wrote:
Il 23/12/24 17:15, Chen-Yu Tsai ha scritto:
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.
MT8173 doesn't have the post processing enabled.... if you can test that the code actually works for MT8173 as well, giving meaningful results, then we can just use the MT8173 compatible :-)
What I'm saying is (and hopefully I regenerated my thoughts correctly) that if we want to get rid of the generic compatible string, then we need a base fallback string. The MT8173 one would be the one.
I think I had misunderstood your statement... so:
For the previously supported ones (including MT8186), yes, I agree.
The MT8186 efuse at the hardware level AFAIK is no different to the MT8173 one. They simply store bits. How the bits are allocated is defined by the layout. How the bits are interpreted is also not a property of the hardware.
I confirm, there's no difference.
In the current design the need for post-processing does not depend on the SoC itself, but rather the node name being "gpu-speedbin". Even if we turned on post processing for all models, it wouldn't matter for the other SoCs.
It wouldn't, but in the event that another SoC needs a different algorithm for post-processing (hopefully not!!!) I would still like the node name to actually be consistent between all of them, even if the algo is different.
The gpu-speedbin efuse would always be gpu-speedbin, regardless of any post-processing differences.
And the reason the conversion is in the nvmem driver (the provider) rather than panfrost (the consumer) has nothing to do with the hardware.
That's correct.
IIRC you simply wanted to provide a generic implementation.
And yes, that's why the conversion is in the nvmem driver, of course.
If the conversion were in the GPU driver, we wouldn't special case MT8186 here.
We would special case MediaTek SoCs in the GPU driver instead, but then the GPU driver is supposed to deal with GPU stuff, not with SoC-specific stuff whenever this is possible - I'm sure you agree with that, anyway.
If the speedbin value were split into two or more cells, we probably would have stuck the conversion in the GPU driver.
Not sure about that, I think there's still a way, but it's all hypotetical for now, so let's not cross any bridges before coming to them :-)
So, we could have "mediatek,mt8173-efuse" replace "mediatek,efuse" as the fallback compatible, and turn on post processing for it, and things would be the same as before, and also work for MT8188. How does that sound?
If MT8173 has (PowerVR!) GPU speedbin efuses, and that requires the same post-processing algorithm as MT8186 and the others, I agree with turning on the post processing for MT8173 with the same algo as MT8186.
Otherwise, I disagree, as that would effectively add faulty code to MT8173.
Interestingly, the new "fixed layout" description actually allows for compatible strings in the NVMEM cells. That would allow matching against a cell's compatible string instead of its name.
If the post-processing stuff ever becomes complicated in the future (in the sense that we start getting more and more post-processing and/or more and more algo differences between SoCs), that'd probably save us some headaches and would most probably be cleaner (at least to the eye) than what we're doing right now.
Though, I'm not sure if it'd be a good idea to consider doing this right now... or the other way around...
Cheers! Angelo
Thanks ChenYu
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 */}, >>> }; >>
linux-stable-mirror@lists.linaro.org
-
AngeloGioacchino Del Regno -
Chen-Yu Tsai