[Letux-kernel] [PATCH 00/20] A bunch of JZ4730 fixups for letux-kernel

[Paul Boddie]

On Monday, 11 January 2021 21:35:03 CET H. Nikolaus Schaller wrote:
> Am 11.01.2021 um 19:50 schrieb Paul Boddie <paul at boddie.org.uk>:
> > 
> > I don't think I ever found out what the "correct" solution was for this.
> 
> Well, for drivers which have a ".compatible" entry they can apparently set
> it in their get_modes function. The panel_simple driver is some mixture.
> If the panel is explicitly listed in the tables, there is a bus format.
> 
> But the fallback was forgotten to somehow provide one. The same is for
> the connector-type. IMHO both should also be defined in the DT.

It is incredible how complicated this is for what is effectively a table of 
values associated with the panel itself. I seem to remember investigating what 
device tree did support, and I think the following is still relevant:

----

I now see that there is code in the driver to read timings for non-DPI panels 
(panel_simple_parse_panel_timing_node). However, I don't see a way of 
specifying a generic "panel-simple" and have the driver activated on that 
basis. None of the Chunghwa panels in the file are comparable to the one used 
by the Mipsbook.

The panel_dpi_probe function reads the timings plus width and height from the 
device tree. In contrast, panel_simple_parse_panel_timing_node only reads the 
timings. Both functions supposedly calculate the appropriate bus flags. I have 
no idea at the moment how the appropriate bus format is obtained. For this 
panel, we need to end up with MEDIA_BUS_FMT_RGB565_1X16, I think.

----

[...]

> IMHO doing it where I have hacked the panel_simple is indeed the right
> place - because the table based initialization does it there as well -
> but it should not be a constant in code.

I should take a look at this, but I now cannot find the details of it in your 
messages.

[...]

> I have checked what is going on and there is another discrepancy. In
> ingenic_drm_plane_config() the fourcc is DRM_FORMAT_RGB888 (no idea where
> this comes from - not from the panel driver where I define
> MEDIA_BUS_FMT_RGB565_1X16) which sets the BPP field to 0x05. The jz4730
> does not permit such a value for this register. Only the jz4740 and later
> SoC version. Otherwise they are almost compatible, at least I have not seen
> another incompatibility.

The JZ4730 supports at most 16bpp displays. There may well be other 
differences, but I haven't got round to documenting the LCD controller in the 
JZ4730 just yet.

> The other issue (which may be the reason for the atomic timeout) is that
> LCDCMD0 seems not to be initialized properly. Therefore the DMA probably
> gets stuck and no SOF/EOF interrupts occur.

There should be a descriptor set up which then populates LCDCMD0 when the DMA 
is activated. It is interesting that the LCD controller supports descriptor-
based transfers whereas the general DMA controller in the JZ4730 does not.

(I have been investigating the DMA controller and have demonstrated it working 
under L4Re, so I will now try and apply the lessons to the Linux driver.)

> At the moment I wouldn't mind if the framebuffer is not displayed properly
> (e.g. shows false colors, distorted or whatever) but the LCDC should scan
> the panel from the framebuffer (something should blink or change if I type)
> and there should be no warnings and errors during boot...
> 
> It seems as if we are close to get this hacked... Then we can look for a
> general solution that does not disturb e.g. jz4780.

Indeed.

> PS: I get an idea where the wrong DRM_FORMAT_RGB888 comes from. There is
> a new format table since v5.11-rc1 which defines the formats every Ingenic
> SoC understands. And I have taken the same table as for the jz4740.
> If user-space or the /dev/fb0 driver can access this table it assumes
> that the jz4730 also understands DRM_FORMAT_XRGB8888. I'll try to add
> a specific jz4730 format table later...

The JZ4730 supports 16bpp as RGB565 and, maybe only for passive displays, 
RGB555 (with the top bit unused). Something comparable to these are defined in 
drivers/gpu/drm/ingenic/ingenic-drm-drv.c alongside a 32bpp format:

static const u32 ingenic_drm_primary_formats[] = {
        DRM_FORMAT_XRGB1555,
        DRM_FORMAT_RGB565,
        DRM_FORMAT_XRGB8888,
};

You previously noted some unconditional access to IPU registers occurring in 
the driver. I think this is the maintainer getting ahead of himself again, 
which may also have happened with the OSD registers. There is no IPU in the 
JZ4730, and support for it probably varies considerably from the earliest SoCs 
to the later ones.

Paul