Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Thu, 31 Dec 2020 00:24:30 +0100
Message-ID: <CAFBinCA4EnbJZ5HnWzqP-LdLJE0FD_nGqA6PxdQaR5KfETgvTQ@mail.gmail.com>
Subject: discussion: re-structuring of the Amlogic Meson VPU DRM driver
To:     Neil Armstrong <narmstrong@baylibre.com>
Cc:     linux-amlogic@lists.infradead.org, dri-devel@lists.freedesktop.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        airlied@linux.ie, daniel@ffwll.ch
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Neil and all interested people,

in the past there were concerns about how some of the components are
coupled in our Meson DRM driver(s).
With this discussion I would like to achieve four things:
1. understand the current issues that we have
2. come up with a TODO list of things that need to be tackled as well
as recommendations how to solve it (for example: "driver ABC function
ABC uses the recommended way - take that as reference")
3. one by one work on the items on the TODO list
4. add support for the 32-bit SoCs to the Meson VPU DRM driver
(without adding more "not recommended" code)

Disclaimer: I am not familiar with the DRM subsystem - so apologies if
the terminology is not correct.

drivers/gpu/drm/meson/meson_dw_hdmi.c currently serves four purposes:
1. manage the TOP (glue) registers for the dw-hdmi IP
This is Amlogic specific and consists of things like HPD filtering,
some internal resets, etc.
In my opinion this part is supposed to stay in this driver

2. load the driver for the dw-hdmi IP by calling dw_hdmi_probe()
I read somewhere that this is not recommended anymore and should be replaced.
Is my understanding correct and what is the recommended replacement?

3. it manages the HDMI PHY registers in the HHI register area
For the 32-bit SoCs I will not follow this pattern and will create a
separate PHY instead.
As a long-term goal I think we should also move this into a dedicated
PHY driver.

4. call back into VPU/VENC functions to set up these registers
This is a blocker for 32-bit SoC support as I would have to duplicate
this code if we don't make any changes. This includes things like
calculating (and setting) clock frequencies, calling
meson_venc_hdmi_mode_set for setting up the DVI pixel encoder, etc.
My understanding is that this part should not be part of the
meson_dw_hdmi driver, but "some other" driver. I don't understand
which driver that's supposed to be though and how things would be
wired up in the end.

In addition to HDMI my understanding is that for adding MIPI DSI
support you would
a) either have to follow the pattern from the meson_dw_hdmi driver or
b) also require some better way to achieve this

The biggest question marks for me are #2 and #4 from the list above.
Also is there anything I have missed?
Any input, feedback and questions are welcome!

PS: an additional topic on the TODO list will be "use the common clock
framework" for clock setup. it's currently not clear to me if that's
possible on the 64-bit SoCs in all cases.
I will start a separate discussion for that topic after this one.


Best regards,
Martin