Hi Andre,
On Tue, 6 Apr 2021 at 13:09, Andre Przywara andre.przywara@arm.com wrote:
On Sat, 6 Mar 2021 20:54:30 +0100 Jernej Skrabec jernej.skrabec@siol.net wrote:
Hi Jernej,
(CC:ing Simon for some DM issues below)
Currently DE2 driver is probed via driver info. Switch probing to device tree compatible string method.
Display is now searched via driver name which has same limitation as previous method. This can be improved only when all drivers in chain are probed via device tree compatible strings.
So on a first glance this looks alright (also the fixed version on your github). However it doesn't work on the Pine64-LTS (or A64 in general), and I identified at least two problems below:
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
drivers/video/sunxi/sunxi_de2.c | 88 +++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 36 deletions(-)
diff --git a/drivers/video/sunxi/sunxi_de2.c b/drivers/video/sunxi/sunxi_de2.c index e02d359cd259..81576e45e9ef 100644 --- a/drivers/video/sunxi/sunxi_de2.c +++ b/drivers/video/sunxi/sunxi_de2.c @@ -31,6 +31,11 @@ enum { LCD_MAX_LOG2_BPP = VIDEO_BPP32, };
+struct sunxi_de2_data {
int id;nit: can you rename this to something less general, like mux or pipeline_id?
const char *disp_drv_name;+};
static void sunxi_de2_composer_init(void) { struct sunxi_ccm_reg * const ccm = @@ -228,51 +233,34 @@ static int sunxi_de2_init(struct udevice *dev, ulong fbbase,
static int sunxi_de2_probe(struct udevice *dev) {
const struct sunxi_de2_data *data =(const struct sunxi_de2_data *)dev_get_driver_data(dev); struct video_uc_plat *plat = dev_get_uclass_plat(dev); struct udevice *disp;
int ret;
int ret, index = 0; /* Before relocation we don't need to do anything */ if (!(gd->flags & GD_FLG_RELOC)) return 0;
ret = uclass_get_device_by_driver(UCLASS_DISPLAY,DM_DRIVER_GET(sunxi_lcd), &disp);if (!ret) {int mux;
while (!(ret = uclass_get_device(UCLASS_DISPLAY, index++, &disp))) {So this call tries to enumerate all devices registered as UCLASS_DISPLAY. For the A64 the sunxi_lcd driver provides one device, however it only probes correctly when there is bridge configured (for the Pinebook, for instance). There is code to read timings from DT, but I don't find any of the required properties (simple-panel) in any Allwinner DTs (using DE2, at least). As a consequence the probe in sunxi_lcd.c fails (except for the Pinebook), and it returns -ENODEV. This is unfortunately the same error code that uclass_get_device_by_driver() returns when there are no (more) devices providing UCLASS_DISPLAY, so the while loop stops here, before having considered the HDMI devices (which are enumerated *after* the LCD device). This is the same problem with your change to use uclass_id_foreach_dev(), btw.
Well, good to check the error to something different. The -ENODEV error is reserved for driver model. It can be returned by the bind() method, but not probe().
Not sure how to best solve this, it sounds like a general DM related problem (failing probe() ending iterating all devices in one class).
That's intended though. I suppose we could change it. But you can iterate through devices with uclass_first_device_err() if you want to do them all..
Is this in core DM code or somewhere else?
I hacked this a bit (using a different error code in sunxi_lcd_probe() and catching/translating this here), but this revealed another problem: I only ever see the mixer0 device being probed (actually multiple times). I could debug that both mixers are detected in the DT, created as two devices and bound (using the right .id value and the right DT offset), but only the first one is ever probed through this function here. Unfortunately this is the wrong one (HDMI is on mixer1), so de2_probe() fails :-( Disabling mixer0 in the DT makes it work (on top of the hack above).
No real clue about this second problem, but I will debug further.
[..]
Regards, Simon