[Letux-kernel] [PATCH 8/8] drm/ingenic: Attach bridge chain to encoders

[Paul Boddie]

On Saturday, 28 August 2021 22:44:36 CEST H. Nikolaus Schaller wrote:
> > Am 28.08.2021 um 21:09 schrieb H. Nikolaus Schaller <hns at goldelico.com>:
> > 
> >> Am 10.08.2021 um 23:15 schrieb Paul Boddie <paul at boddie.org.uk>:
> >> 
> >> Although I had fancy ideas about finding the connector node in the device
> >> tree and populating the necessary type details on the bridge, I decided
> >> to just add the following to the Synopsys driver's dw_hdmi_probe
> >> function:
> >> 
> >> hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> >> 
> >> This then lets the above invocation succeed and the Ingenic DRM driver
> >> initialises. However, I get "Input Not Supported" on my display.
> > 
> > Finally I am back to testing (on top of 5.14-rc7 now) and can confirm the
> > symptoms and that this line above has a visible influence: I get a green
> > screen.
> > 
> > This seems to indicate that the lcdc is scanning and the hdmi driver core
> > is working.

I would think so. I seem to remember that my "Input Not Supported" message 
indicates a functioning peripheral and perhaps a functioning LCDC, but they 
just aren't functioning together!

> > And there are many drm commit timeouts:
> > 
> > [  390.947342] [drm:drm_crtc_commit_wait [drm]] *ERROR* flip_done timed
> > out
> > [  390.948018] [drm:drm_atomic_helper_wait_for_dependencies
> > [drm_kms_helper]] *ERROR* [CRTC:32:crtc-0] commit wait timed out [ 
> > 401.187362] [drm:drm_crtc_commit_wait [drm]] *ERROR* flip_done timed out
> > [  401.188023] [drm:drm_atomic_helper_wait_for_dependencies
> > [drm_kms_helper]] *ERROR* [CONNECTOR:35:HDMI-A-1] commit wait timed out [
> >  411.427371] [drm:drm_crtc_commit_wait [drm]] *ERROR* flip_done timed out
> > [  411.428029] [drm:drm_atomic_helper_wait_for_dependencies
> > [drm_kms_helper]] *ERROR* [PLANE:31:plane-0] commit wait timed out [ 
> > 411.537372] ------------[ cut here ]------------
> > 
> > Most likely it has something to do with the extended DMA descriptors which
> > I also had modified as suggested by review.

I also saw these, I think.

> > I tried to fix like Paul C. suggested but with no success. The list of
> > bridge drivers seems to be empty when calling of_drm_find_bridge() in
> > dw_hdmi_probe().
> Here is what Paul C. did write:
> 
> From what I understand the last bridge in the chained list is supposed to
> set the connector type. The HDMI driver's probe function should get a
> pointer to the next bridge in the queue and attach it (see how
> ite-it66121.c does it). The last bridge in the queue should be
> "hdmi-connector" (display-connector.c) which will effectively set the
> connector type.
> > So the dw-hdmi driver does not find the connector.
> > 
> > The code (fragment) is
> > 
> > 	ep = of_graph_get_remote_node(dev->of_node, 1, -1);
> > 	if (!ep)
> > 	
> > 			return ERR_PTR(-EPROBE_DEFER);
> > 	
> > 	hdmi->next_bridge = of_drm_find_bridge(ep);
> > 	of_node_put(ep);
> > 
> > but hdmi->next_bridge == NULL.

I can't remember exactly what is missing right now, but I did notice that the 
Synopsys driver doesn't find the connector. I suspected that the different 
specialisations of the driver might be setting something, or they might be 
initialising the driver in a slightly different way, but I concluded that I 
needed a way of setting the connector myself, and there didn't seem to be an 
operation I could "override" specifically for that purpose.

Since it was probably getting late and I had spent long enough looking at this 
code, I just thought that the easiest option would be to short-circuit the 
problem and just set the connector to what we know it should be. That was 
enough to complete the chain. Ideally, our specialisation would look up the 
connector in the device tree and do effectively the same thing. I just got 
tired of working with the weird data structures.

> > So we have to solve two separate issues:
> > a) no video data (descriptors?)
> 
> Hm. Must be something else since upstream-jz4780-hdmi-v3 ([PATCH v3 0/9]
> MIPS: JZ4780 and CI20 HDMI) already included all these changes and was
> working!
> > b) no correct setup of hdmi->bridge.type
> 
> Maybe both are the same issue, i.e. with Paul C's patches the setup is
> incomplete.
> > But wait - I think I still have to rebase things to v2 of the series of
> > Paul C.
> Done - makes no difference.

It is always possible that some inaccuracy or discrepancy has crept in.

> Next was to check lcdc setup
> 
> a) with new code (upstream-jz4780-hdmi-v4)
> 
> ./jzlcdc
> 0x13050000 12f00000 JZ_REG_LCD_CFG
> 0x13050004 00000003 JZ_REG_LCD_VSYNC
> 0x13050008 0000000d JZ_REG_LCD_HSYNC
> 0x1305000c 0520026e JZ_REG_LCD_VAT
> 0x13050010 011b051b JZ_REG_LCD_DAH
> 0x13050014 0014026c JZ_REG_LCD_DAV
> 0x13050018 00000000 JZ_REG_LCD_PS
> 0x1305001c 00000000 JZ_REG_LCD_CLS
> 0x13050020 00000000 JZ_REG_LCD_SPL
> 0x13050024 00000000 JZ_REG_LCD_REV
> 0x13050030 24002008 JZ_REG_LCD_CTRL
> 0x13050034 00000004 JZ_REG_LCD_STATE
> 0x13050038 00000000 JZ_REG_LCD_IID
> 0x13050040 0559e000 JZ_REG_LCD_DA0
> 0x13050044 00000000 JZ_REG_LCD_SA0
> 0x13050048 000000f0 JZ_REG_LCD_FID0
> 0x1305004c 00000000 JZ_REG_LCD_CMD0
> 0x13050050 0559e010 JZ_REG_LCD_DA1
> 0x13050054 01000000 JZ_REG_LCD_SA1
> 0x13050058 000000f1 JZ_REG_LCD_FID1
> 0x1305005c 40096000 JZ_REG_LCD_CMD1
> 
> b) with old code (upstream-jz4780-hdmi-v3 - how good that I am running an
> archive of old branch versions - editing history is bad practise but
> required for linux development :)
> 
> 0x13050000 12f00040 JZ_REG_LCD_CFG
> 0x13050004 00000003 JZ_REG_LCD_VSYNC
> 0x13050008 0000000d JZ_REG_LCD_HSYNC
> 0x1305000c 0520026e JZ_REG_LCD_VAT
> 0x13050010 011b051b JZ_REG_LCD_DAH
> 0x13050014 0014026c JZ_REG_LCD_DAV
> 0x13050018 00000000 JZ_REG_LCD_PS
> 0x1305001c 00000000 JZ_REG_LCD_CLS
> 0x13050020 00000000 JZ_REG_LCD_SPL
> 0x13050024 00000000 JZ_REG_LCD_REV
> 0x13050030 24000008 JZ_REG_LCD_CTRL
> 0x13050034 00000026 JZ_REG_LCD_STATE
> 0x13050038 000000f0 JZ_REG_LCD_IID
> 0x13050040 05353000 JZ_REG_LCD_DA0
> 0x13050044 00000000 JZ_REG_LCD_SA0
> 0x13050048 000000f0 JZ_REG_LCD_FID0
> 0x1305004c 00000000 JZ_REG_LCD_CMD0
> 0x13050050 05353010 JZ_REG_LCD_DA1
> 0x13050054 01300000 JZ_REG_LCD_SA1
> 0x13050058 000000f1 JZ_REG_LCD_FID1
> 0x1305005c 44096000 JZ_REG_LCD_CMD1
> 
> Well, there are minor diffs to be analysed...

These being in terms of old versus new:

JZ_REG_LCD_CFG   12f00040 vs. 12f00000

Here, the 24-bit colour flag is missing in the new value, which seems rather 
undesirable.

JZ_REG_LCD_CTRL  24000008 vs. 24002008

Here, only the EOFM flag is different, with the new value having it set and 
thus actually enabling the interrupt.

JZ_REG_LCD_STATE 00000026 vs. 00000004

Here, the old value indicates EOF, IFU0 and IFU1 (end of frame, input FIFO #0 
and #1 underflow), whereas the new value just indicates IFU0.

JZ_REG_LCD_IID   000000f0 vs. 00000000

This should reproduce the frame identifier from the descriptor when the EOF 
interrupt occurs (or maybe the condition rather than an actual interrupt), 
suggesting that no such interrupts have been generated (or maybe no frames 
have been generated).

The descriptor address registers (JZ_REG_LCD_DA0 and JZ_REG_LCD_DA1) are 
different, but this is not unexpected. Similarly, the source address registers 
(JZ_REG_LCD_SA0 and JZ_REG_LCD_SA1) are different.

One thing that my L4Re code does, following the legacy driver, is to enable 
both DMA channels, but I see that this isn't done above. However, it may be 
unnecessary, particularly if the old code whose configuration is shown above 
manages to work satisfactorily.

The first thing I would check is the configuration register and that 24-bit 
flag. If this isn't being set because of a bus format miscommunication, then 
that would be the problem to solve.

Paul