[PATCH v2 09/19] video: sunxi: de2: switch to public uclass functions

Jernej Škrabec jernej.skrabec at siol.net
Tue Mar 9 06:35:53 CET 2021


Dne torek, 09. marec 2021 ob 01:40:32 CET je Andre Przywara napisal(a):
> On Sun, 07 Mar 2021 08:35:23 +0100
> Jernej Škrabec <jernej.skrabec at siol.net> wrote:
> 
> Hi,
> 
> > Dne nedelja, 07. marec 2021 ob 02:32:52 CET je Andre Przywara napisal(a):
> > > On Sat,  6 Mar 2021 20:54:27 +0100
> > > 
> > > Jernej Skrabec <jernej.skrabec at siol.net> wrote:
> > > > Currently DE2 driver uses functions which are defined in internal
> > > > headers. They are not meant to be used outside of uclass framework.
> > > > Switch DE2 driver to public ones. This has additional benefit that
> > > > device_probe doesn't need to be called manually.
> > > 
> > > Indeed, good solution: for the calls in _probe(), we call de2_init()
> > > right afterwards, which would explicitly call probe for the display, so
> > > this both saves this call and the usage of the internal function.
> > > For the calls in simplefb_setup: the DM framework checks if a device
> > > has already been activated, so there is no problem with double probes.
> > > 
> > > However, actually testing this on a Pine64-LTS revealed that this breaks
> > > on the A64: I only get a black screen (bisecting down to this commit).
> > > 
> > > I didn't investigate any further yet, just wanted to give a heads up.
> > 
> > Thanks! I'll check what's going on.
> 
> I think I know what's going on, see below.
> 
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec at siol.net>
> > > > ---
> > > > 
> > > >  drivers/video/sunxi/sunxi_de2.c | 29 ++++++++++-------------------
> > > >  1 file changed, 10 insertions(+), 19 deletions(-)
> > > > 
> > > > diff --git a/drivers/video/sunxi/sunxi_de2.c
> > > > b/drivers/video/sunxi/sunxi_de2.c index 6b836a011944..e02d359cd259
> > > > 100644
> > > > --- a/drivers/video/sunxi/sunxi_de2.c
> > > > +++ b/drivers/video/sunxi/sunxi_de2.c
> > > > @@ -19,8 +19,6 @@
> > > > 
> > > >  #include <asm/io.h>
> > > >  #include <asm/arch/clock.h>
> > > >  #include <asm/arch/display2.h>
> > > > 
> > > > -#include <dm/device-internal.h>
> > > > -#include <dm/uclass-internal.h>
> > > > 
> > > >  #include <linux/bitops.h>
> > > >  #include "simplefb_common.h"
> > > > 
> > > > @@ -198,13 +196,6 @@ static int sunxi_de2_init(struct udevice *dev,
> > > > ulong
> > > > fbbase,>
> > > > 
> > > >  	disp_uc_plat->source_id = mux;
> 
> The mux value is set *after* the call to uclass_find_device_by_name()
> (or uclass_get_device_by_driver()) returned, and then we call this
> function with it, which *used* to call probe().
> Now probe is already called as part of uclass_get_device_by_driver(),
> so *before* we had a chance to determine and set the mux value/
> source_id.
> For the H3/H5 it's 0, so it conveniently works with the probably zeroed
> uc_plat structure there.

Oh, thanks for looking into this!

> 
> Not sure how to fix this, as we get a handle to the device only after
> the uclass_find/get functions returned, so don't know where to set
> source_id before that.
> 
> Shall this H3 vs. A64 check be moved into the HDMI driver and it's probe
> routine?

This info shouldn't be needed once TCON is implemented as standalone driver 
probed via compatible string. I guess workaround can be implemented in HDMI 
driver to get TCON number from DT instead, although this is a bit clumsy.

Best regards,
Jernej

> 
> Cheers,
> Andre
> 
> > > > -	ret = device_probe(disp);
> > > > -	if (ret) {
> > > > -		debug("%s: device '%s' display won't probe (ret=%d)
\n",
> > > > -		      __func__, dev->name, ret);
> > > > -		return ret;
> > > > -	}
> > > > -
> > > > 
> > > >  	ret = display_read_timing(disp, &timing);
> > > >  	if (ret) {
> > > >  	
> > > >  		debug("%s: Failed to read timings\n", __func__);
> > > > 
> > > > @@ -245,8 +236,8 @@ static int sunxi_de2_probe(struct udevice *dev)
> > > > 
> > > >  	if (!(gd->flags & GD_FLG_RELOC))
> > > >  	
> > > >  		return 0;
> > > > 
> > > > -	ret = uclass_find_device_by_name(UCLASS_DISPLAY,
> > > > -					 "sunxi_lcd", &disp);
> > > > +	ret = uclass_get_device_by_driver(UCLASS_DISPLAY,
> > > > +
> > 
> > DM_DRIVER_GET(sunxi_lcd), &disp);
> > 
> > > >  	if (!ret) {
> > > >  	
> > > >  		int mux;
> > > > 
> > > > @@ -262,8 +253,8 @@ static int sunxi_de2_probe(struct udevice *dev)
> > > > 
> > > >  	debug("%s: lcd display not found (ret=%d)\n", __func__, ret);
> > > > 
> > > > -	ret = uclass_find_device_by_name(UCLASS_DISPLAY,
> > > > -					 "sunxi_dw_hdmi",
> > 
> > &disp);
> > 
> > > > +	ret = uclass_get_device_by_driver(UCLASS_DISPLAY,
> > > > +
> > 
> > DM_DRIVER_GET(sunxi_dw_hdmi), &disp);
> > 
> > > >  	if (!ret) {
> > > >  	
> > > >  		int mux;
> > > >  		if (IS_ENABLED(CONFIG_MACH_SUNXI_H3_H5))
> > > > 
> > > > @@ -332,8 +323,8 @@ int sunxi_simplefb_setup(void *blob)
> > > > 
> > > >  		mux = 1;
> > > >  	
> > > >  	/* Skip simplefb setting if DE2 / HDMI is not present */
> > > > 
> > > > -	ret = uclass_find_device_by_name(UCLASS_VIDEO,
> > > > -					 "sunxi_de2", &de2);
> > > > +	ret = uclass_get_device_by_driver(UCLASS_VIDEO,
> > > > +
> > 
> > DM_DRIVER_GET(sunxi_de2), &de2);
> > 
> > > >  	if (ret) {
> > > >  	
> > > >  		debug("DE2 not present\n");
> > > >  		return 0;
> > > > 
> > > > @@ -342,8 +333,8 @@ int sunxi_simplefb_setup(void *blob)
> > > > 
> > > >  		return 0;
> > > >  	
> > > >  	}
> > > > 
> > > > -	ret = uclass_find_device_by_name(UCLASS_DISPLAY,
> > > > -					 "sunxi_dw_hdmi",
> > 
> > &hdmi);
> > 
> > > > +	ret = uclass_get_device_by_driver(UCLASS_DISPLAY,
> > > > +
> > 
> > DM_DRIVER_GET(sunxi_dw_hdmi), &hdmi);
> > 
> > > >  	if (ret) {
> > > >  	
> > > >  		debug("HDMI not present\n");
> > > >  	
> > > >  	} else if (device_active(hdmi)) {
> > > > 
> > > > @@ -355,8 +346,8 @@ int sunxi_simplefb_setup(void *blob)
> > > > 
> > > >  		debug("HDMI present but not probed\n");
> > > >  	
> > > >  	}
> > > > 
> > > > -	ret = uclass_find_device_by_name(UCLASS_DISPLAY,
> > > > -					 "sunxi_lcd", &lcd);
> > > > +	ret = uclass_get_device_by_driver(UCLASS_DISPLAY,
> > > > +
> > 
> > DM_DRIVER_GET(sunxi_lcd), &lcd);
> > 
> > > >  	if (ret)
> > > >  	
> > > >  		debug("LCD not present\n");
> > > >  	
> > > >  	else if (device_active(lcd))






More information about the U-Boot mailing list