Date:   Sat, 09 May 2020 15:43:21 +0200
From:   Paul Cercueil <paul@...pouillou.net>
To:     Paul Boddie <paul@...die.org.uk>
Cc:     "H. Nikolaus Schaller" <hns@...delico.com>,
        Dave Airlie <airlied@...ux.ie>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Andy Yan <andy.yan@...k-chips.com>,
        Yakir Yang <ykk@...k-chips.com>,
        Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>,
        linux-mips@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        MIPS Creator CI20 Development 
        <mips-creator-ci20-dev@...glegroups.com>
Subject: Re: DRM interaction problems on Ingenic CI20 / jz4780 with dw-hdmi
 and ingenic-drm

Hi Paul,

Le mar. 5 mai 2020 à 20:26, Paul Boddie <paul@...die.org.uk> a écrit :
> On Monday 4. May 2020 03.05.22 Paul Cercueil wrote:
>> 
>>  > Le sam. 11 avril 2020 à 16:14, H. Nikolaus Schaller 
>> <hns@...delico.com> a
>>  > écrit :
>>  >>
>>  >> So far we have identified two issues.
>>  >>
>>  >> The first is that HPD interrupts are not properly processed.
>>  >>
>>  >> drm_helper_hpd_irq_event() is called by HPD events but
>>  >> dev->mode_config.poll_enabled is false.
>> 
>>  This is to be used when there's no hardware interrupt. I believe you
>>  have one, right? Then call drm_kms_helper_hotplug_event() from the
>>  interrupt handler instead.
> 
> What we have in drivers/gpu/drm/bridge/synopsys/dw-hdmi.c is a 
> function called
> dw_hdmi_irq which appears to be the thread_fn for HDMI interrupts 
> (alongside
> the dw_hdmi_hardirq which is the handler), initialised by a call to
> devm_request_threaded_irq.
> 
> In dw_hdmi_irq, a hotplug event seems to cause 
> drm_helper_hpd_irq_event to be
> called. However...
> 
> [...]
> 
>>  >> The jz4780 hdmi subsystem (drm/bridge/dw-hdmi.c) uses
>>  >>
>>  >> 	connector->polled = DRM_CONNECTOR_POLL_HPD;
>>  >>
>>  >> but shouldn't this enable polling? Note that there seems to be
>>  >> no (direct) call to drm_kms_helper_poll_init().
>>  >>
>>  >> If we set dev->mode_config.poll_enabled = true in
>>  >> drm_helper_hpd_irq_event() things start to work.
>>  >>
>>  >> Please can you clarify what would be best practise here to
>>  >> get HPD event handling working.
>> 
>>  Remove that - this stuff is for hardware without interrupts, where
>>  everything has to be polled.
> 
> Yes, I think this must be a mistake in the driver. In
> drm_helper_hpd_irq_event, the connector is tested for the
> DRM_CONNECTOR_POLL_HPD flag and skips the connector. It isn't clear 
> whether
> this actually matters for the other hardware using this technology,
> documentation being rather thin on the ground.
> 
>>  >> The other issue is in dw-hdmi.c:
>>  >>
>>  >> We found out that ingenic_drm_encoder_atomic_check() fails 
>> because
>>  >>
>>  >> info->num_bus_formats == 0
>>  >>
>>  >> and not 1. This blocks further initialization.
>>  >>
>>  >> The reason seems to be that dw_hdmi_bridge_attach() does not call
>>  >> drm_display_info_set_bus_formats() with a proper format like
>>  >> other drivers (e.g. drm/bridge/ti-tfp410.c) are doing.
>>  >>
>>  >> We have patched to set a single bus format 
>> MEDIA_BUS_FMT_RGB888_1X24
>>  >> and then DRM setup seems to work (although we still have no valid
>>  >> HDMI signal but that is likely something else).
>>  >>
>>  >> Please can you explain how setting the bus format should be fixed
>>  >> in dw-hdmi.c.
>> 
>>  I'm not sure, but that information may come from EDID data. Are you
>>  able to obtain video modes from the connected monitor?
> 
> The modes are definitely received, or at least the list of modes 
> given by
> /sys/devices/platform/13050000.lcd/drm/card0/card0-HDMI-A-1/modes is 
> viable.
> 
> However, it rather looked like the bus format information wasn't 
> being set and
> that this inhibited the completion of the initialisation process 
> which, if
> completed, would ultimately cause the format to be set. (This being 
> the short
> version of the story as I remember it right now.) So, the problem 
> presents
> itself as an initialisation order problem.

It's not an initialization order problem, the ingenic-drm expects the 
bus_formats to be provided and the synopsis driver never calls 
drm_display_info_set_bus_formats().

> I removed the flag from the connector to see if it makes any 
> difference, but
> it doesn't look like it. Here is what /sys/kernel/debug/dri/0/state 
> contains:
> 
> plane[31]: plane-0
>         crtc=(null)
>         fb=0
>         crtc-pos=0x0+0+0
>         src-pos=0.000000x0.000000+0.000000+0.000000
>         rotation=1
>         normalized-zpos=0
>         color-encoding=ITU-R BT.601 YCbCr
>         color-range=YCbCr limited range
> crtc[32]: crtc-0
>         enable=0
>         active=0
>         self_refresh_active=0
>         planes_changed=0
>         mode_changed=0
>         active_changed=0
>         connectors_changed=0
>         color_mgmt_changed=0
>         plane_mask=0
>         connector_mask=0
>         encoder_mask=0
>         mode: "": 0 0 0 0 0 0 0 0 0 0 0x0 0x0
> connector[34]: HDMI-A-1
>         crtc=(null)
>         self_refresh_aware=0
> 
> The crtc member values do not look encouraging. In fact, it just 
> looks like
> most structure members are uninitialised.
> 
> Thanks for the advice: I spent some time the other day reviewing 
> various
> aspects of the Synopsys drivers of different vintages (Ingenic 3.0.8 
> non-DRM
> driver for JZ4780, MIPS/Ingenic 3.18 DRM driver for JZ4780 based on 
> Freescale
> driver code, the recent generic DRM bridge driver), and so this 
> information is
> timely and valuable.

With a hardcoded bus_format of RGB888 in the ingenic-drm driver, and 
jz4780_dw_hdmi_plat_data.input_bus_format initialized too:

modetest -D /dev/dri/card0 -M ingenic-drm -s 35@32:1280x720-60

My PC monitor sees a signal - unfortunately it says "input not 
supported".


> Paul
> 
> P.S. Sorry if this message goes to far too many people. I don't want 
> to
> "personalise" it by taking people off the recipients list, but I 
> realise that
> this is probably not interesting to most recipients, either. Feel 
> free to trim
> recipients if replying.

Again, you shoul send this message to the DRI mailing list. The people 
who did work with the Synopsis IP may be able to help.

-Paul

Powered by blists - more mailing lists