Re: [PATCH v1 17/17] drm/mediatek: Add mt8195-dpi support to drm_drv

[Krzysztof Kozlowski]

On 03/10/2022 17:29, Guillaume Ranquet wrote:
> On Tue, 27 Sep 2022 16:28, Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>> On 27/09/2022 15:04, Guillaume Ranquet wrote:
>>> On Thu, 22 Sep 2022 09:20, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
>>>> On 19/09/2022 18:56, Guillaume Ranquet wrote:
>>>>> Add dpi support to enable the HDMI path.
>>>>>
>>>>> Signed-off-by: Guillaume Ranquet <granquet@xxxxxxxxxxxx>
>>>>>
>>>>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>>> index 72049a530ae1..27f029ca760b 100644
>>>>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>>>>> @@ -820,6 +820,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
>>>>>  	  .data = (void *)MTK_DPI },
>>>>>  	{ .compatible = "mediatek,mt8192-dpi",
>>>>>  	  .data = (void *)MTK_DPI },
>>>>> +	{ .compatible = "mediatek,mt8195-dpi",
>>>>> +	  .data = (void *)MTK_DPI },
>>>>
>>>> It's compatible with the others. You don't need more compatibles.
>>>
>>> Hi Krzysztof,
>>>
>>> It's a bit confusing, because this compatible is used in both
>>> mtk_drm_drv.c and in mtk_dpi.c
>>>
>>> Albeit it's entirely the same thing regarding the mtk_drm_drv module,
>>> it's pretty different
>>> regarding the mtk_dpi module.
>>
>> Sure, but this does not explain why do you need these entries here in
>> mtk_drm_drv.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Hi Krzysztof,
> 
> Sorry for the late answer.
> The mtk_drm_drv is the component master of the full mediatek drm stack.
> 
> it "binds" all of the crtc/dpi/ovl/mutex/merge... components of the stack.
> 
> That mtk_ddp_comp_dt_ids array is iterated over to find all of the components
> from the device tree.

No. You said what the code is doing. I think I understand this. You
still do not need more compatibles. Your sentence did not clarify it
because it did not answer at all to question "why". Why do you need it?

Sorry, the change looks not correct.

Best regards,
Krzysztof