[v7,01/13] dt-bindings: arm: move mmsys description to display
diff mbox series

Message ID 20200213201953.15268-2-matthias.bgg@kernel.org
State Changes Requested
Headers show
Series
  • arm/arm64: mediatek: Fix mmsys device probing
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Matthias Brugger Feb. 13, 2020, 8:19 p.m. UTC
From: Matthias Brugger <mbrugger@suse.com>

The mmsys block provides registers and clocks for the display
subsystem. The binding description should therefore live together with
the rest of the display descriptions. Move it to display/mediatek.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

---

Changes in v7:
- move the binding description

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt         | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (100%)

Comments

CK Hu Feb. 14, 2020, 6:42 a.m. UTC | #1
Hi, Matthias:

On Thu, 2020-02-13 at 21:19 +0100, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> The mmsys block provides registers and clocks for the display
> subsystem. The binding description should therefore live together with
> the rest of the display descriptions. Move it to display/mediatek.
> 

Yes, for the upstreamed driver, only display (DRM) use mmsys clock. For
some MDP patches [1] in progress, MDP also use mmsys clock. So we just
consider what's upstreamed now?

[1] https://patchwork.kernel.org/patch/11140747/

Regards,
CK

> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> 
> ---
> 
> Changes in v7:
> - move the binding description
> 
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt         | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (100%)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
> similarity index 100%
> rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> rename to Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
Matthias Brugger Feb. 14, 2020, 10:01 a.m. UTC | #2
On 14/02/2020 07:42, CK Hu wrote:
> Hi, Matthias:
> 
> On Thu, 2020-02-13 at 21:19 +0100, matthias.bgg@kernel.org wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> The mmsys block provides registers and clocks for the display
>> subsystem. The binding description should therefore live together with
>> the rest of the display descriptions. Move it to display/mediatek.
>>
> 
> Yes, for the upstreamed driver, only display (DRM) use mmsys clock. For
> some MDP patches [1] in progress, MDP also use mmsys clock. So we just
> consider what's upstreamed now?

I'm not sure if I understand you correctly. Are you proposing to keep the
binding description in arm/mediatek?

Regards,
Matthias

> 
> [1] https://patchwork.kernel.org/patch/11140747/
> 
> Regards,
> CK
> 
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>
>> ---
>>
>> Changes in v7:
>> - move the binding description
>>
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>>
>>  .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt         | 0
>>  1 file changed, 0 insertions(+), 0 deletions(-)
>>  rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (100%)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
>> similarity index 100%
>> rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>> rename to Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Enric Balletbo i Serra Feb. 14, 2020, 12:19 p.m. UTC | #3
Hi CK,

On 14/2/20 11:01, Matthias Brugger wrote:
> 
> 
> On 14/02/2020 07:42, CK Hu wrote:
>> Hi, Matthias:
>>
>> On Thu, 2020-02-13 at 21:19 +0100, matthias.bgg@kernel.org wrote:
>>> From: Matthias Brugger <mbrugger@suse.com>
>>>
>>> The mmsys block provides registers and clocks for the display
>>> subsystem. The binding description should therefore live together with
>>> the rest of the display descriptions. Move it to display/mediatek.
>>>
>>
>> Yes, for the upstreamed driver, only display (DRM) use mmsys clock. For
>> some MDP patches [1] in progress, MDP also use mmsys clock. So we just
>> consider what's upstreamed now?
> 

Let me jump into the discussion, and sorry if my question is silly because I'm
just starting to look at this code.

IMO we should consider all the cases to find a proper fix on all this, and if
MDP uses also mmsys clocks this approach will not work. I think the main problem
here and the big question is what exactly is the MMSYS block, is an independent
clock controller that provides clocks to DRM and other blocks? or is hardly tied
to the DRM block in some way?

Could you give us a block schema on how the things are interconnected?

If is an independent clock controller I think there was a mistake when the first
drm driver was pushed by using the compatible = "mediatek,mt8173-mmsys" as id
for that driver.

Thanks,
 Enric


> I'm not sure if I understand you correctly. Are you proposing to keep the
> binding description in arm/mediatek?
> 
> Regards,
> Matthias
> 
>>
>> [1] https://patchwork.kernel.org/patch/11140747/
>>
>> Regards,
>> CK
>>
>>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>>
>>> ---
>>>
>>> Changes in v7:
>>> - move the binding description
>>>
>>> Changes in v6: None
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>>  .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt         | 0
>>>  1 file changed, 0 insertions(+), 0 deletions(-)
>>>  rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (100%)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
>>> similarity index 100%
>>> rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>>> rename to Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
CK Hu Feb. 14, 2020, 4:22 p.m. UTC | #4
Hi, Matthias & Enric:

On Fri, 2020-02-14 at 13:19 +0100, Enric Balletbo i Serra wrote:
> Hi CK,
> 
> On 14/2/20 11:01, Matthias Brugger wrote:
> > 
> > 
> > On 14/02/2020 07:42, CK Hu wrote:
> >> Hi, Matthias:
> >>
> >> On Thu, 2020-02-13 at 21:19 +0100, matthias.bgg@kernel.org wrote:
> >>> From: Matthias Brugger <mbrugger@suse.com>
> >>>
> >>> The mmsys block provides registers and clocks for the display
> >>> subsystem. The binding description should therefore live together with
> >>> the rest of the display descriptions. Move it to display/mediatek.
> >>>
> >>
> >> Yes, for the upstreamed driver, only display (DRM) use mmsys clock. For
> >> some MDP patches [1] in progress, MDP also use mmsys clock. So we just
> >> consider what's upstreamed now?
> > 
> 
> Let me jump into the discussion, and sorry if my question is silly because I'm
> just starting to look at this code.
> 
> IMO we should consider all the cases to find a proper fix on all this, and if
> MDP uses also mmsys clocks this approach will not work. I think the main problem
> here and the big question is what exactly is the MMSYS block, is an independent
> clock controller that provides clocks to DRM and other blocks? or is hardly tied
> to the DRM block in some way?
> 
> Could you give us a block schema on how the things are interconnected?
> 
> If is an independent clock controller I think there was a mistake when the first
> drm driver was pushed by using the compatible = "mediatek,mt8173-mmsys" as id
> for that driver.
> 

I correct my mistake first. In mt8173, mdp has already upstreamed [1].

There are many partitions in Mediatek SoC. mmsys is one of these
partition. There are many function blocks in mmsys such as OVL, RDMA,
RSZ, WROT, .... Some data routing between these blocks are fixed but
some are changeable. For application, we group them into display path
and mdp path. Clock gating register of these blocks are in the range of
0x14000000 ~ 0x14000fff. The routing control register of these blocks
are also in the range of 0x14000000 ~ 0x14000fff. So the control
function belong to mmsys partition but not belong to specific function
block would in the register range of 0x14000000 ~ 0x14000fff. I think
there could be two definition of mmsys device. One is that mmsys device
is the whole mmsys partiotion, so OVL, RDMA, ... would be sub device of
it. Another is that mmsys just control register of 0x14000000 ~
0x14000fff, so it's part of mmsys partition like OVL, RDMA, .....
Currently we define mmsys as the latter one. I've no idea how to map
mmsys into current Linux hardware category, but I think it is not just a
display device.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.6-rc1

Regards,
CK

> Thanks,
>  Enric
> 
> 
> > I'm not sure if I understand you correctly. Are you proposing to keep the
> > binding description in arm/mediatek?
> > 
> > Regards,
> > Matthias
> > 
> >>
> >> [1] https://patchwork.kernel.org/patch/11140747/
> >>
> >> Regards,
> >> CK
> >>
> >>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> >>>
> >>> ---
> >>>
> >>> Changes in v7:
> >>> - move the binding description
> >>>
> >>> Changes in v6: None
> >>> Changes in v5: None
> >>> Changes in v4: None
> >>> Changes in v3: None
> >>> Changes in v2: None
> >>>
> >>>  .../bindings/{arm => display}/mediatek/mediatek,mmsys.txt         | 0
> >>>  1 file changed, 0 insertions(+), 0 deletions(-)
> >>>  rename Documentation/devicetree/bindings/{arm => display}/mediatek/mediatek,mmsys.txt (100%)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
> >>> similarity index 100%
> >>> rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
> >>> rename to Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt
similarity index 100%
rename from Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
rename to Documentation/devicetree/bindings/display/mediatek/mediatek,mmsys.txt