From: Paul Cercueil <paul@crapouillou.net>
To: Paul Boddie <paul@boddie.org.uk>
Cc: "H. Nikolaus Schaller" <hns@goldelico.com>,
Dave Airlie <airlied@linux.ie>,
Sascha Hauer <s.hauer@pengutronix.de>,
Andy Yan <andy.yan@rock-chips.com>,
Yakir Yang <ykk@rock-chips.com>,
Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>,
linux-mips@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
MIPS Creator CI20 Development
<mips-creator-ci20-dev@googlegroups.com>
Subject: Re: DRM interaction problems on Ingenic CI20 / jz4780 with dw-hdmi and ingenic-drm
Date: Sat, 09 May 2020 15:43:21 +0200 [thread overview]
Message-ID: <9SG2AQ.ALB8O9UHXRT11@crapouillou.net> (raw)
In-Reply-To: <79437309.Fv5JZGVDrq@jeremy>
Hi Paul,
Le mar. 5 mai 2020 à 20:26, Paul Boddie <paul@boddie.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@goldelico.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
prev parent reply other threads:[~2020-05-09 13:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-11 14:14 H. Nikolaus Schaller
2020-04-13 11:25 ` Paul Cercueil
2020-04-13 11:46 ` H. Nikolaus Schaller
2020-05-04 1:05 ` Paul Cercueil
2020-05-05 18:26 ` Paul Boddie
2020-05-09 13:43 ` Paul Cercueil [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9SG2AQ.ALB8O9UHXRT11@crapouillou.net \
--to=paul@crapouillou.net \
--cc=airlied@linux.ie \
--cc=andy.yan@rock-chips.com \
--cc=hns@goldelico.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=mips-creator-ci20-dev@googlegroups.com \
--cc=paul@boddie.org.uk \
--cc=s.hauer@pengutronix.de \
--cc=vladimir_zapolskiy@mentor.com \
--cc=ykk@rock-chips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®