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

[H. Nikolaus Schaller]

Hi Paul,

> Am 14.01.2021 um 00:29 schrieb Paul Boddie <paul at boddie.org.uk>:
> 
> On Wednesday, 13 January 2021 18:17:56 CET H. Nikolaus Schaller wrote:
>> 
>> Seems as if I have some breaktrough!!!
>> 
>> I managed to play with demmem2 to suddenly make the boot log visible on the
>> LCD.
>> 
>> It seems as if I have to provide the second DMA descriptor (also) as the
>> first. Maybe the first descriptor is broken.
> 
> Looking at the drivers/gpu/drm/ingenic/ingenic-drm-drv.c file, I see this in 
> ingenic_drm_plane_atomic_update:
> 
>  if (priv->soc_info->has_osd && plane->type == DRM_PLANE_TYPE_OVERLAY)
>          hwdesc = &priv->dma_hwdescs->hwdesc_f0;
>  else
>          hwdesc = &priv->dma_hwdescs->hwdesc_f1;

Yes,exactly.

I hacked 

> hwdesc = &priv->dma_hwdescs->hwdesc_f0;

after that line and now I get interrupts and no atomic flush timeouts any more.
But still no vertical scanning of the display.

Only the console login: is stuck. This may have something to do with my
kernel command line and the user-space which wants to do a getty on the
screen console. This may hang the boot process.

But we have again made another step forward...

> 
> With the JZ4740 (and JZ4730), this will always set up the second descriptor 
> because they do not have the OSD functionality.
> 
> Meanwhile, in ingenic_drm_bind:
> 
>  if (soc_info->has_osd) {
>          drm_plane_helper_add(&priv->f0,
>                               &ingenic_drm_plane_helper_funcs);
> 
>          ret = drm_universal_plane_init(drm, &priv->f0, 1,
>                                         &ingenic_drm_primary_plane_funcs,
>                                         priv->soc_info->formats_f0,
>                                         priv->soc_info->num_formats_f0,
>                                         NULL, DRM_PLANE_TYPE_OVERLAY,
>                                         NULL);
> 
> I wonder whether the first descriptor would ever get set up on this hardware. 
> (I also wonder about how this code would interact with the JZ4780, but that is 
> another matter.)
> 
> Looking at the log, I see a commit that might explain some of the breakage:
> 
> ----
> commit 174d8e52a60f196cfbdec299688cf14f124c8b67
> Author: Paul Cercueil <paul at crapouillou.net>
> Date:   Sat Sep 26 19:04:57 2020 +0200
> 
>    drm/ingenic: Alloc F0 and F1 DMA descriptors at once
> 
>    Instead of calling dmam_alloc_coherent() once for each 4-bit DMA
>    hardware descriptor, we can have them both in a physical memory page, as
>    long as they are aligned to 16 bytes. This reduces memory consumption,
>    and will make it easier to add more DMA descriptors in the future.
> 
>    Note that the old code would not create the F0 descriptor on SoCs that
>    don't support multiple planes. We don't care, because:
>    - we don't use more memory by allocating two descriptors instead of a
>      single one;
>    - the only SoC that does not support multiple planes (JZ4740) still has
>      two independent DMA channels, for an unknown reason.

I also wondered what the second descriptor is doing.

What may be is that there are two descriptors because the LCDC can support
two LCDs?

Maybe the programming manual and the observed behaviour gives another hint.

> 
>    Signed-off-by: Paul Cercueil <paul at crapouillou.net>
>    Reviewed-by: Sam Ravnborg <sam at ravnborg.org>
>    Link: https://patchwork.freedesktop.org/patch/msgid/
> 20200926170501.1109197-4-paul at crapouillou.net
> ----
> 
> Incidentally, related to something I noted before, the JZ4740 employs a two-
> descriptor arrangement to support display modes that use a palette, albeit 
> using one DMA channel. However, dual-panel support involves two DMA channels, 
> but with the all constant churn, the code that supported this is probably long 
> since forgotten in Linux, hence the "unknown reason" for that second channel.

Ok, it may indeed be the palette. If I remember the 2.6.24 code there was
some setup for a palette descriptor.

> 
> Back in ingenic_drm_bind...
> 
>  regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_phys_f0);
>  regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_phys_f1);
> 
> Here, the only descriptor that is likely to be initialised on the JZ4740 is 
> the one whose physical address is written to the second channel, but the 
> second channel is for the second panel (which we do not have).
> 
> Perhaps the easiest hack is to just do something like this:
> 
>  if (!soc_info->has_osd) {
>          regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_phys_f0);
>          regmap_write(priv->map, JZ_REG_LCD_DA1, dma_hwdesc_phys_f1);
>  } else {
>          regmap_write(priv->map, JZ_REG_LCD_DA0, dma_hwdesc_phys_f1);
>  }
> 
> That isn't very nice, but "f0" and "f1" do not exist as such in the older 
> SoCs, and if "f0" ends up being dedicated to OSD usage then "f1" will need to 
> find its way to the appropriate channel register.

If the palettte theory is correct, we may have to add another bool to the
SoC descriptor.

> 
>> I also had provided in that experiment my other devmem2 commands which
>> change the pixel clock but next I can try what happens if I change it back.
>> 
>> The dev/tty isn't working because the DRM system isn't aware that I now
>> have enabled the LCDC...
> 
> Hopefully, we can figure that out in the end.
> 
>> This raises the question how the code wants to handle the f0 and f1
>> descriptors. Especially it appears if the f1 descriptor is good but the LCD
>> is connected to the DMA channel for the f0 descriptor.
>> 
>> A hint is that the new const struct jz_soc_info has fields for different
>> format tables for f0 and f1. Strangely the jz4740 defines only f1 and I
>> have simply copied it. Maybe that is the bug...
> 
> I think we just need to make sure that if the first descriptor is not set up, 
> then we just ignore it and use the second one. At this point, I regard the 
> driver as being broken for the JZ4740 unless I am missing something at this 
> rather late point in a long day.

Indeed. I also wonder about the jz4740...

So let's make the jz4730 work and send a patch and ask the same question
for the jz4740 to the maintainers :)

BR,
Nikolaus