linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "H. Nikolaus Schaller" <hns@goldelico.com>
To: Paul Cercueil <paul@crapouillou.net>
Cc: Paul Boddie <paul@boddie.org.uk>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	linux-mips <linux-mips@vger.kernel.org>,
	list@opendingux.net, dri-devel <dri-devel@lists.freedesktop.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 6/6] drm/ingenic: Attach bridge chain to encoders
Date: Sat, 25 Sep 2021 21:26:42 +0200	[thread overview]
Message-ID: <96585ED9-B707-4AF1-8417-E03DE6414965@goldelico.com> (raw)
In-Reply-To: <HU700R.NAHL5IU3NRE81@crapouillou.net>

Hi Paul,

> Am 25.09.2021 um 21:08 schrieb Paul Cercueil <paul@crapouillou.net>:
> 
> Hi Paul & Nikolaus,
> 
> If you spent some time debugging the issue

we did ...

> instead of complaining that my patchset breaks things...

... we did have a working version (without hdmi-connector)
and bisect pointed at your patch... So we debugged that.

So the lesson is: don't trust bisect.

And failed to make it work with hdmi-connector because the
ingenic-drm-drv reported errors.

> 
> The fix is a one-liner in your downstream ingenic-dw-hdmi.c:
> .output_port = 1
> in the ingenic_dw_hdmi_plat_data struct.

Cool. How did you find that?

> 
> Absolutely nothing else needs to be changed for HDMI to work here.

Great and thanks.

Will test asap and if it works as well, we can clean up a v4 patch set
for next week review.

BR and thanks,
Nikolaus

> 
> Cheers,
> -Paul
> 
> 
> Le sam., sept. 25 2021 at 17:55:03 +0200, Paul Boddie <paul@boddie.org.uk> a écrit :
>> On Friday, 24 September 2021 10:29:02 CEST Paul Cercueil wrote:
>>> Le ven., sept. 24 2021 at 00:51:39 +0200, Paul Boddie
>>> >
>>> > 2. My approach, which just involves changing the Synopsys driver to
>>> > set the bridge type in dw_hdmi_probe like this:
>>> >
>>> >   hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>>> >
>>> > Otherwise, I don't see how the bridge's (struct drm_bridge) type will
>>> > be set.
>>> The bridge's type is set in hdmi-connector, from DTS. The 'type = "a"'
>>> will result in the bridge's .type to be set to DRM_MODE_CONNECTOR_HDMIA.
>> Actually, I found that hdmi-connector might not have been available because
>> CONFIG_DRM_DISPLAY_CONNECTOR was not enabled. Rectifying this, the connector
>> does get detected and enabled. However, the Synopsys driver remains unaware of
>> it, and so the bridge type in the Synopsys driver remains unset.
>> I do see that the connector sets the type on a bridge in its own private
>> structure, so there would be a need to propagate this type to the actual
>> bridge. In other words, what the connector does is distinct from the Synopsys
>> driver which acts as the bridge with regard to the Ingenic driver.
>> Perhaps the Synopsys driver should set the connector's bridge as the next
>> bridge, or maybe something is supposed to discover that the connector may act
>> as (or provide) a bridge after the Synopsys driver in the chain and then back-
>> propagate the bridge type along the chain.
>> [...]
>>> > And I removed any of the above hacks. What I observe, apart from an
>>> > inactive LCD controller (and ingenic-drm driver), is the following in
>>> > /sys/devices/platform/10180000.hdmi/:
>>> >
>>> > consumer:platform:13050000.lcdc0
>>> > consumer:platform:hdmi_connector
>> Interestingly, with the connector driver present, these sysfs entries no
>> longer appear.
>> [...]
>>> > For me, running modetest yields plenty of information about encoders,
>>> > connectors (and the supported modes via the EDID, thanks to my HDMI-A
>>> > hack), CRTCs, and planes. But no framebuffers are reported.
>>> Could you paste the result of "modetest -a -c -p" somewhere maybe?
>> I had to specify -M ingenic-drm as well, but here you go...
>> ----
>> Connectors:
>> id	encoder	status		name		size (mm)	modes	encoders
>> 35	34	connected	HDMI-A-1       	340x270		17	34
>>  modes:
>> 	index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot)
>>  #0 1280x1024 60.02 1280 1328 1440 1688 1024 1025 1028 1066 108000 flags:
>> phsync, pvsync; type: preferred, driver
>>  #1 1280x1024 75.02 1280 1296 1440 1688 1024 1025 1028 1066 135000 flags:
>> phsync, pvsync; type: driver
>>  #2 1280x960 60.00 1280 1376 1488 1800 960 961 964 1000 108000 flags: phsync,
>> pvsync; type: driver
>>  #3 1152x864 75.00 1152 1216 1344 1600 864 865 868 900 108000 flags: phsync,
>> pvsync; type: driver
>>  #4 1024x768 75.03 1024 1040 1136 1312 768 769 772 800 78750 flags: phsync,
>> pvsync; type: driver
>>  #5 1024x768 70.07 1024 1048 1184 1328 768 771 777 806 75000 flags: nhsync,
>> nvsync; type: driver
>>  #6 1024x768 60.00 1024 1048 1184 1344 768 771 777 806 65000 flags: nhsync,
>> nvsync; type: driver
>>  #7 832x624 74.55 832 864 928 1152 624 625 628 667 57284 flags: nhsync,
>> nvsync; type: driver
>>  #8 800x600 75.00 800 816 896 1056 600 601 604 625 49500 flags: phsync,
>> pvsync; type: driver
>>  #9 800x600 72.19 800 856 976 1040 600 637 643 666 50000 flags: phsync,
>> pvsync; type: driver
>>  #10 800x600 60.32 800 840 968 1056 600 601 605 628 40000 flags: phsync,
>> pvsync; type: driver
>>  #11 800x600 56.25 800 824 896 1024 600 601 603 625 36000 flags: phsync,
>> pvsync; type: driver
>>  #12 640x480 75.00 640 656 720 840 480 481 484 500 31500 flags: nhsync,
>> nvsync; type: driver
>>  #13 640x480 72.81 640 664 704 832 480 489 492 520 31500 flags: nhsync,
>> nvsync; type: driver
>>  #14 640x480 66.67 640 704 768 864 480 483 486 525 30240 flags: nhsync,
>> nvsync; type: driver
>>  #15 640x480 59.94 640 656 752 800 480 490 492 525 25175 flags: nhsync,
>> nvsync; type: driver
>>  #16 720x400 70.08 720 738 846 900 400 412 414 449 28320 flags: nhsync,
>> pvsync; type: driver
>>  props:
>> 	1 EDID:
>> 		flags: immutable blob
>> 		blobs:
>> 		value:
>> 			00ffffffffffff00047232ad01010101
>> 			2d0e010380221b782aaea5a6544c9926
>> 			145054bfef0081808140714f01010101
>> 			010101010101302a009851002a403070
>> 			1300520e1100001e000000ff00343435
>> 			3030353444454330300a000000fc0041
>> 			4c313731350a202020202020000000fd
>> 			00384c1e520e000a2020202020200051
>> 	2 DPMS:
>> 		flags: enum
>> 		enums: On=0 Standby=1 Suspend=2 Off=3
>> 		value: 0
>> 	5 link-status:
>> 		flags: enum
>> 		enums: Good=0 Bad=1
>> 		value: 0
>> 	6 non-desktop:
>> 		flags: immutable range
>> 		values: 0 1
>> 		value: 0
>> 	4 TILE:
>> 		flags: immutable blob
>> 		blobs:
>> 		value:
>> 	20 CRTC_ID:
>> 		flags: object
>> 		value: 32
>> CRTCs:
>> id	fb	pos	size
>> 32	39	(0,0)	(1280x1024)
>>  #0  60.02 1280 1328 1440 1688 1024 1025 1028 1066 108000 flags: phsync,
>> pvsync; type:
>>  props:
>> 	22 ACTIVE:
>> 		flags: range
>> 		values: 0 1
>> 		value: 1
>> 	23 MODE_ID:
>> 		flags: blob
>> 		blobs:
>> 		value:
>> 			e0a5010000053005a005980600000004
>> 			010404042a0400003c00000005000000
>> 			00000000000000000000000000000000
>> 			00000000000000000000000000000000
>> 			00000000
>> 	19 OUT_FENCE_PTR:
>> 		flags: range
>> 		values: 0 18446744073709551615
>> 		value: 0
>> 	24 VRR_ENABLED:
>> 		flags: range
>> 		values: 0 1
>> 		value: 0
>> 	28 GAMMA_LUT:
>> 		flags: blob
>> 		blobs:
>> 		value:
>> 	29 GAMMA_LUT_SIZE:
>> 		flags: immutable range
>> 		values: 0 4294967295
>> 		value: 256
>> Planes:
>> id	crtc	fb	CRTC x,y	x,y	gamma size	possible crtcs
>> 31	32	39	0,0		0,0	0       	0x00000001
>>  formats: XR15 RG16 RG24 XR24 XR30
>>  props:
>> 	8 type:
>> 		flags: immutable enum
>> 		enums: Overlay=0 Primary=1 Cursor=2
>> 		value: 1
>> 	17 FB_ID:
>> 		flags: object
>> 		value: 39
>> 	18 IN_FENCE_FD:
>> 		flags: signed range
>> 		values: -1 2147483647
>> 		value: -1
>> 	20 CRTC_ID:
>> 		flags: object
>> 		value: 32
>> 	13 CRTC_X:
>> 		flags: signed range
>> 		values: -2147483648 2147483647
>> 		value: 0
>> 	14 CRTC_Y:
>> 		flags: signed range
>> 		values: -2147483648 2147483647
>> 		value: 0
>> 	15 CRTC_W:
>> 		flags: range
>> 		values: 0 2147483647
>> 		value: 1280
>> 	16 CRTC_H:
>> 		flags: range
>> 		values: 0 2147483647
>> 		value: 1024
>> 	9 SRC_X:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 0
>> 	10 SRC_Y:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 0
>> 	11 SRC_W:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 83886080
>> 	12 SRC_H:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 67108864
>> 33	0	0	0,0		0,0	0       	0x00000001
>>  formats: C8   XR15 RG16 RG24 XR24 XR30
>>  props:
>> 	8 type:
>> 		flags: immutable enum
>> 		enums: Overlay=0 Primary=1 Cursor=2
>> 		value: 0
>> 	17 FB_ID:
>> 		flags: object
>> 		value: 0
>> 	18 IN_FENCE_FD:
>> 		flags: signed range
>> 		values: -1 2147483647
>> 		value: -1
>> 	20 CRTC_ID:
>> 		flags: object
>> 		value: 0
>> 	13 CRTC_X:
>> 		flags: signed range
>> 		values: -2147483648 2147483647
>> 		value: 0
>> 	14 CRTC_Y:
>> 		flags: signed range
>> 		values: -2147483648 2147483647
>> 		value: 0
>> 	15 CRTC_W:
>> 		flags: range
>> 		values: 0 2147483647
>> 		value: 0
>> 	16 CRTC_H:
>> 		flags: range
>> 		values: 0 2147483647
>> 		value: 0
>> 	9 SRC_X:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 0
>> 	10 SRC_Y:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 0
>> 	11 SRC_W:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 0
>> 	12 SRC_H:
>> 		flags: range
>> 		values: 0 4294967295
>> 		value: 0
>> ----
>>> If you have info about the CRTCs, encoders, connectors and EDID info,
>>> then I would assume it is very close to working fine.
>>> For your "no framebuffer" issue, keep in mind that CONFIG_FB and
>>> CONFIG_FRAMEBUFFER_CONSOLE are now disabled by default.
>> Yes, I discovered that CONFIG_FB was not enabled, so I did so.
>>> If that doesn't fix anything, that probably means that one
>>> .atomic_check() fails, so it would be a good place to start debugging.
>> There will be other things to verify in the Ingenic driver. As noted many
>> months ago, colour depth information has to be set in the DMA descriptors and
>> not the control register, but we are managing to do this successfully, as far
>> as I can tell, although there is always the potential for error.
>> Paul
> 
> 


  reply	other threads:[~2021-09-25 19:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22 20:55 [PATCH v3 0/6] drm/ingenic: Various improvements v3 Paul Cercueil
2021-09-22 20:55 ` [PATCH v3 1/6] drm/ingenic: Simplify code by using hwdescs array Paul Cercueil
2021-09-22 20:55 ` [PATCH v3 2/6] drm/ingenic: Add support for private objects Paul Cercueil
2021-09-22 20:55 ` [PATCH v3 3/6] drm/ingenic: Move IPU scale settings to private state Paul Cercueil
2021-09-22 20:55 ` [PATCH v3 4/6] drm/ingenic: Set DMA descriptor chain register when starting CRTC Paul Cercueil
2021-09-22 20:55 ` [PATCH v3 5/6] drm/ingenic: Upload palette before frame Paul Cercueil
2021-09-22 20:55 ` [PATCH v3 6/6] drm/ingenic: Attach bridge chain to encoders Paul Cercueil
2021-09-23  5:52   ` H. Nikolaus Schaller
2021-09-23  8:49     ` Paul Cercueil
2021-09-23  9:19       ` H. Nikolaus Schaller
2021-09-23  9:27         ` Laurent Pinchart
2021-09-23  9:55           ` H. Nikolaus Schaller
2021-09-23 10:03             ` Laurent Pinchart
2021-09-23 11:41               ` H. Nikolaus Schaller
2021-09-23 11:56                 ` H. Nikolaus Schaller
2021-09-23 13:30                 ` Paul Cercueil
     [not found]                   ` <ABE75744-46FE-4F37-A14C-D996F36B7B0E@goldelico.com>
2021-09-23 19:39                     ` Paul Cercueil
2021-09-23 20:23                       ` H. Nikolaus Schaller
2021-09-23 22:51                         ` Paul Boddie
2021-09-24  8:29                           ` Paul Cercueil
2021-09-25 15:55                             ` Paul Boddie
2021-09-25 19:08                               ` Paul Cercueil
2021-09-25 19:26                                 ` H. Nikolaus Schaller [this message]
2021-09-25 19:39                                   ` Paul Cercueil
2021-09-27 16:18                                     ` H. Nikolaus Schaller
2021-09-24 11:40                         ` H. Nikolaus Schaller
2021-09-23  9:23       ` Laurent Pinchart

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=96585ED9-B707-4AF1-8417-E03DE6414965@goldelico.com \
    --to=hns@goldelico.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=list@opendingux.net \
    --cc=paul@boddie.org.uk \
    --cc=paul@crapouillou.net \
    /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;
as well as URLs for NNTP newsgroup(s).