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

[H. Nikolaus Schaller]

Hi Paul,

> Am 29.08.2021 um 16:35 schrieb H. Nikolaus Schaller <hns at goldelico.com>:
> 
> Hi Paul,
> just found that we don't have CONFIG_DRM_DISPLAY_CONNECTOR configured...
> Maybe the v3 code simply works without *any* connector setup - like our hack.
> 
> Now, the connector driver is found and registered and of_drm_find_bridge()
> no longer returns NULL. I.e. my patch according to Paul C's suggestion seems
> to work better.
> 
> It still does not work because dw_hdmi_bridge_attach() is called with
> flags = DRM_BRIDGE_ATTACH_NO_CONNECTOR... but that can be analysed by
> some dump_stack();

Ok, comparing again with Paul C's suggestion to look how ite-it66121.c
is working has revealed something interesting:

The dw-hdmi.c creates its own connector through dw_hdmi_connector_create().

I.e. the whole approach to go through the device tree and look up
the connector and chain it is wrong.

> BTW: it looks as if there is some code duplication for detecting
> "ddc-i2c-bus" in dw-hdmi.c and in display-connector.c We have neither
> one defined in our device trees and use the internal driver through
> dw_hdmi_i2c_adapter().

Ok, now this all makes sense...

There is no provision for the dw-hdmi to make use of display-connector.
Because it has its own.

Therefore not configuring CONFIG_DRM_DISPLAY_CONNECTOR is right.
Checking locally for "ddc-i2c-bus" is right.
Not going through the device tree to locate the connector and prepare
some next_bridge chain is right.

Unless we completely refactor dw-hdmi to use display-connector...

So the solution may become:
* remove the connector from the device tree (it isn't used unless we
heavily rework dw-hdmi).
* set the connector type in dw_hdmi_connector_create() or dw_hdmi_bridge_attach()
* make dw_hdmi_connector_create() called at all...
  This may add bus format detection etc.

I.e. we have been completely on the wrong track for 2 days...

But the real problem seems to be

ingenic_drm_bind()
...
		ret = drm_bridge_attach(encoder, &ib->bridge, NULL,
					DRM_BRIDGE_ATTACH_NO_CONNECTOR);
		if (ret) {
			dev_err(dev, "Unable to attach bridge\n");
			return ret;
		}

		connector = drm_bridge_connector_init(drm, encoder);
		if (IS_ERR(connector)) {
			dev_err(dev, "Unable to init connector: err = %d\n", PTR_ERR(connector));
			return PTR_ERR(connector);
		}

		drm_connector_attach_encoder(connector, encoder);

and

dw_hdmi_bridge_attach()
...

	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
		return 0;

So ingenic_drm_bind() will
1. call dw_hdmi_bridge_attach(flags = DRM_BRIDGE_ATTACH_NO_CONNECTOR)
2. this returns 0
3. no error...
4. drm_bridge_connector_init()
5. fails

The drm_bridge_connector_init() call in ingenic_drm_bind() is
new code by Paul C's commit 8804b44506062.

So Paul C's code assumes that there is a connector. While dw-hdmi
assumes there is none.

At least we understand the problem now and why it was not present in -v3.

Probably we should confront Paul C that his latest ingenic-drm series
is incompatble with the dw-hdmi code (not our ingenic specialization).
So he should add some fall-back for our case.

Do you remember which other devices use dw-hdmi?

BR,
Nikolaus