There are several dt-binding error for MT8183 and MT8192. Therefore, we send this series to correct them. 1. Correct dt-binding and dts for MT8183. 2. Correct dt-binding for MT8192.
This series is based on this series: "add display support for MediaTek SoC MT8186": Message id: 20220406030009.2357-1-rex-bc.chen@mediatek.com
Rex-BC Chen (3): dt-bindings: display: mediatek: Correct disp_aal binding for MT8183 arm64: dts: mt8183: Correct disp_aal node compatible dt-bindings: display: mediatek: Correct disp_aal binding for MT8192
.../bindings/display/mediatek/mediatek,aal.yaml | 8 ++++---- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-)
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com --- .../devicetree/bindings/display/mediatek/mediatek,aal.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml index cb4a2f39ff88..f118075e4a89 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml @@ -21,12 +21,12 @@ description: | properties: compatible: oneOf: - - items: - - const: mediatek,mt8173-disp-aal + - enum: + - mediatek,mt8173-disp-aal + - mediatek,mt8183-disp-aal - items: - enum: - mediatek,mt2712-disp-aal - - mediatek,mt8183-disp-aal - mediatek,mt8192-disp-aal - mediatek,mt8195-disp-aal - const: mediatek,mt8173-disp-aal
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Hello! Yes, I definitely agree with this change, it makes no sense to have both 8173 and 8183 compatibles for 8183 (and similar).
Though, please, add a note in this commit message advertising that the mistake was only in the yaml binding, as the driver doesn't require any functional changes, so that things are clear for people with less expertise on MediaTek platforms.
After that: Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
On Wed, 2022-04-06 at 12:38 +0200, AngeloGioacchino Del Regno wrote:
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Hello! Yes, I definitely agree with this change, it makes no sense to have both 8173 and 8183 compatibles for 8183 (and similar).
Though, please, add a note in this commit message advertising that the mistake was only in the yaml binding, as the driver doesn't require any functional changes, so that things are clear for people with less expertise on MediaTek platforms.
After that: Reviewed-by: AngeloGioacchino Del Regno < angelogioacchino.delregno@collabora.com>
Hello Angelo,
Thanks for your review. I will add them to the commit message in next version.
BRs, Rex
On 06/04/2022 11:46, Rex-BC Chen wrote:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Just because one feature is not supported, it does not mean they are incompatible, which you claim in the patch below. Are you sure they are really incompatible and MT8173 fallback cannot be used?
Best regards, Krzysztof
On Wed, 2022-04-06 at 16:44 +0200, Krzysztof Kozlowski wrote:
On 06/04/2022 11:46, Rex-BC Chen wrote:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Just because one feature is not supported, it does not mean they are incompatible, which you claim in the patch below. Are you sure they are really incompatible and MT8173 fallback cannot be used?
Best regards, Krzysztof
Hello Krzysztof,
Thanks for your review.
The difference of disp_aal for each MediaTek SoCs is "has_gamma". And we only control this variable for different MediaTek SoCs.
The value of has_gamma for MT8173 is true. The value of has_gamma for MT8183 is false. (Moreover, the driver data is null for MT8183)
From this situation, I think it's not compatible between MT8173 and
MT8183.
reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
BRs, Rex
On 07/04/2022 08:22, Rex-BC Chen wrote:
On Wed, 2022-04-06 at 16:44 +0200, Krzysztof Kozlowski wrote:
On 06/04/2022 11:46, Rex-BC Chen wrote:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Just because one feature is not supported, it does not mean they are incompatible, which you claim in the patch below. Are you sure they are really incompatible and MT8173 fallback cannot be used?
Best regards, Krzysztof
Hello Krzysztof,
Thanks for your review.
The difference of disp_aal for each MediaTek SoCs is "has_gamma". And we only control this variable for different MediaTek SoCs.
The value of has_gamma for MT8173 is true. The value of has_gamma for MT8183 is false. (Moreover, the driver data is null for MT8183)
From this situation, I think it's not compatible between MT8173 and MT8183.
You repeated the commit msg without bringing any new information... but let it be, I assume setting gamma on MTT8183 is incorrect or produces wrong results.
Best regards, Krzysztof
On Thu, 2022-04-07 at 08:49 +0200, Krzysztof Kozlowski wrote:
On 07/04/2022 08:22, Rex-BC Chen wrote:
On Wed, 2022-04-06 at 16:44 +0200, Krzysztof Kozlowski wrote:
On 06/04/2022 11:46, Rex-BC Chen wrote:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Just because one feature is not supported, it does not mean they are incompatible, which you claim in the patch below. Are you sure they are really incompatible and MT8173 fallback cannot be used?
Best regards, Krzysztof
Hello Krzysztof,
Thanks for your review.
The difference of disp_aal for each MediaTek SoCs is "has_gamma". And we only control this variable for different MediaTek SoCs.
The value of has_gamma for MT8173 is true. The value of has_gamma for MT8183 is false. (Moreover, the driver data is null for MT8183)
From this situation, I think it's not compatible between MT8173 and MT8183.
You repeated the commit msg without bringing any new information... but let it be, I assume setting gamma on MTT8183 is incorrect or produces wrong results.
Best regards, Krzysztof
Hello Krzysztof,
Sorry for this. I can explain more detailed.
For MT8173, the gamma module is inside disp_aal. When we need to adjust gamma value, we need to use "has_gamma" to control gamma function inside disp_aal to adjust gamma value.
Since MT8183 (MT8192/MT8195/MT8186), display gamma is seperated from disp_aal. We just need to control disp_gamma directly and we don't need to control gamma function inside disp_aal.
I will add these comments to commit message in next version to explain it.
BRs, Rex
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com --- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 4b08691ed39e..a3616005d65c 100644 --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -1466,8 +1466,7 @@ };
aal0: aal@14010000 { - compatible = "mediatek,mt8183-disp-aal", - "mediatek,mt8173-disp-aal"; + compatible = "mediatek,mt8183-disp-aal"; reg = <0 0x14010000 0 0x1000>; interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_LOW>; power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data of MT8183 and MT8173 are different. The value of has_gamma for MT8173 is true while the value of MT8183 is false. Therefore, the compatible of disp_aal for MT8183 is not suitable for the compatible for MT8173.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com --- .../devicetree/bindings/display/mediatek/mediatek,aal.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml index f118075e4a89..407d7cbd6240 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml @@ -27,12 +27,12 @@ properties: - items: - enum: - mediatek,mt2712-disp-aal - - mediatek,mt8192-disp-aal - mediatek,mt8195-disp-aal - const: mediatek,mt8173-disp-aal - items: - enum: - mediatek,mt8186-disp-aal + - mediatek,mt8192-disp-aal - const: mediatek,mt8183-disp-aal
reg:
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
...though, from what I know, you should also move MT8195 down there!
On Wed, 2022-04-06 at 18:40 +0800, AngeloGioacchino Del Regno wrote:
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Reviewed-by: AngeloGioacchino Del Regno < angelogioacchino.delregno@collabora.com>
...though, from what I know, you should also move MT8195 down there!
Hello Angelo,
Thanks for your review. But for MT8195, disp_aal is using the same compatible with MT8173.
From the MT8195 project DTS is using:
aal0: disp_aal@1c005000 { compatible = "mediatek,mt8195-disp-aal", "mediatek,mt8173-disp-aal"; reg = <0 0x1c005000 0 0x1000>; interrupts = <GIC_SPI 641 IRQ_TYPE_LEVEL_HIGH 0>; power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>; clocks = <&vdosys0 CLK_VDO0_DISP_AAL0>; mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x5000 0x1000>; };
BRs, Rex
Il 06/04/22 12:48, Rex-BC Chen ha scritto:
On Wed, 2022-04-06 at 18:40 +0800, AngeloGioacchino Del Regno wrote:
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Reviewed-by: AngeloGioacchino Del Regno < angelogioacchino.delregno@collabora.com>
...though, from what I know, you should also move MT8195 down there!
Hello Angelo,
Thanks for your review. But for MT8195, disp_aal is using the same compatible with MT8173.
From the MT8195 project DTS is using:
aal0: disp_aal@1c005000 { compatible = "mediatek,mt8195-disp-aal", "mediatek,mt8173-disp-aal"; reg = <0 0x1c005000 0 0x1000>; interrupts = <GIC_SPI 641 IRQ_TYPE_LEVEL_HIGH 0>; power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>; clocks = <&vdosys0 CLK_VDO0_DISP_AAL0>; mediatek,gce-client-reg = <&gce0 SUBSYS_1c00XXXX 0x5000 0x1000>; };
BRs, Rex
I'm sorry Rex, I've just realized - I was confusing this one with the 8195 gamma binding instead!
Cheers, Angelo
On Wed, 2022-04-06 at 18:40 +0800, AngeloGioacchino Del Regno wrote:
Il 06/04/22 11:46, Rex-BC Chen ha scritto:
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Signed-off-by: Rex-BC Chen rex-bc.chen@mediatek.com
Reviewed-by: AngeloGioacchino Del Regno < angelogioacchino.delregno@collabora.com>
...though, from what I know, you should also move MT8195 down there!
Hello Angelo,
We have some misunderstanding for disp_aal of MT8195. The MT8195 has independent disp_gamma which is seperated from disp_aal since MT8173. Therefore, for MT8195, the disp_aal should be the same with MT8183. I have checked this with Jason-JH Lin who is drm owner of MT8195.
I will update disp_aal binding in next version.
BRs, Rex
On 06/04/2022 11:46, Rex-BC Chen wrote:
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Driver data is Linux implementation specific so does not really matter for bindings. Probably you want to say here that devices are fully compatible? If so, please rephrase it. Also remove the wording "correct it" in subject and here because there is no mistake here (at least you did not describe it) and otherwise AUTOSEL might pick it up...
Best regards, Krzysztof
On Wed, 2022-04-06 at 22:46 +0800, Krzysztof Kozlowski wrote:
On 06/04/2022 11:46, Rex-BC Chen wrote:
The driver data for MT8192 is the same with MT8183. Therefore, we correct it.
Driver data is Linux implementation specific so does not really matter for bindings. Probably you want to say here that devices are fully compatible? If so, please rephrase it. Also remove the wording "correct it" in subject and here because there is no mistake here (at least you did not describe it) and otherwise AUTOSEL might pick it up...
Best regards, Krzysztof
Hello Krzysztof,
Thanks for your review. Yes, it's not a error for dt-binding. I will update the description in next version.
BRs, Rex
dri-devel@lists.freedesktop.org
-
AngeloGioacchino Del Regno -
Krzysztof Kozlowski -
Rex-BC Chen