| Message ID | 20180328124021.17239-6-mario.six@gdsys.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Mario Six |
| Headers | show |
| Series | [U-Boot,1/7] drivers: Add AXI uclass and ihs_axi driver | expand |
Hi Mario, On 28 March 2018 at 20:40, Mario Six <mario.six@gdsys.cc> wrote: > Add a power_on function to the display uclass to allow devices to be > probed and powered-on separately. Is this different from the 'enable' method? Also note that we have a panel uclass that might be useful. > > Signed-off-by: Mario Six <mario.six@gdsys.cc> > --- > drivers/video/display-uclass.c | 10 ++++++++++ > include/display.h | 18 ++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/video/display-uclass.c b/drivers/video/display-uclass.c > index e752eb07c4..4865057e94 100644 > --- a/drivers/video/display-uclass.c > +++ b/drivers/video/display-uclass.c > @@ -57,6 +57,16 @@ int display_read_timing(struct udevice *dev, struct display_timing *timing) > return edid_get_timing(buf, ret, timing, &panel_bits_per_colour); > } > > +int display_power_on(struct udevice *dev, void *data) > +{ > + struct dm_display_ops *ops = display_get_ops(dev); > + > + if (!ops || !ops->power_on) > + return -ENOSYS; > + > + return ops->power_on(dev, data); > +} > + > bool display_in_use(struct udevice *dev) > { > struct display_plat *disp_uc_plat = dev_get_uclass_platdata(dev); > diff --git a/include/display.h b/include/display.h > index d0a08d4aaa..bb263be246 100644 > --- a/include/display.h > +++ b/include/display.h > @@ -51,6 +51,15 @@ int display_enable(struct udevice *dev, int panel_bpp, > */ > bool display_in_use(struct udevice *dev); > > +/** > + * display_power_on() - Power on display port device > + * > + * @dev: Device to power on > + * @data: Optional data needed to power on the display correctly > + * @return 0 if OK, -ve on error > + */ > +int display_power_on(struct udevice *dev, void *data); > + > struct dm_display_ops { > /** > * read_timing() - Read information directly > @@ -81,6 +90,15 @@ struct dm_display_ops { > */ > int (*enable)(struct udevice *dev, int panel_bpp, > const struct display_timing *timing); > + > + /** > + * power_on() - Power on display port device > + * > + * @dev: Device to power on > + * @data: Optional data needed to power on the display correctly > + * @return 0 if OK, -ve on error > + */ > + int (*power_on)(struct udevice *dev, void *data); > }; > > #define display_get_ops(dev) ((struct dm_display_ops *)(dev)->driver->ops) > -- > 2.16.1 > Regards, Simon
Hi Simon, On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Mario, > > On 28 March 2018 at 20:40, Mario Six <mario.six@gdsys.cc> wrote: >> Add a power_on function to the display uclass to allow devices to be >> probed and powered-on separately. > > Is this different from the 'enable' method? > I was thinking that this is more low-level than the enable method, which also sets display parameters. I could also use the enable method if it's alright to disregard the panel_bpp and timing parameters. > Also note that we have a panel uclass that might be useful. > The Logicore driver is probably more low-level than that, but I'll take a look. >> >> Signed-off-by: Mario Six <mario.six@gdsys.cc> >> --- >> drivers/video/display-uclass.c | 10 ++++++++++ >> include/display.h | 18 ++++++++++++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/drivers/video/display-uclass.c b/drivers/video/display-uclass.c >> index e752eb07c4..4865057e94 100644 >> --- a/drivers/video/display-uclass.c >> +++ b/drivers/video/display-uclass.c >> @@ -57,6 +57,16 @@ int display_read_timing(struct udevice *dev, struct display_timing *timing) >> return edid_get_timing(buf, ret, timing, &panel_bits_per_colour); >> } >> >> +int display_power_on(struct udevice *dev, void *data) >> +{ >> + struct dm_display_ops *ops = display_get_ops(dev); >> + >> + if (!ops || !ops->power_on) >> + return -ENOSYS; >> + >> + return ops->power_on(dev, data); >> +} >> + >> bool display_in_use(struct udevice *dev) >> { >> struct display_plat *disp_uc_plat = dev_get_uclass_platdata(dev); >> diff --git a/include/display.h b/include/display.h >> index d0a08d4aaa..bb263be246 100644 >> --- a/include/display.h >> +++ b/include/display.h >> @@ -51,6 +51,15 @@ int display_enable(struct udevice *dev, int panel_bpp, >> */ >> bool display_in_use(struct udevice *dev); >> >> +/** >> + * display_power_on() - Power on display port device >> + * >> + * @dev: Device to power on >> + * @data: Optional data needed to power on the display correctly >> + * @return 0 if OK, -ve on error >> + */ >> +int display_power_on(struct udevice *dev, void *data); >> + >> struct dm_display_ops { >> /** >> * read_timing() - Read information directly >> @@ -81,6 +90,15 @@ struct dm_display_ops { >> */ >> int (*enable)(struct udevice *dev, int panel_bpp, >> const struct display_timing *timing); >> + >> + /** >> + * power_on() - Power on display port device >> + * >> + * @dev: Device to power on >> + * @data: Optional data needed to power on the display correctly >> + * @return 0 if OK, -ve on error >> + */ >> + int (*power_on)(struct udevice *dev, void *data); >> }; >> >> #define display_get_ops(dev) ((struct dm_display_ops *)(dev)->driver->ops) >> -- >> 2.16.1 >> > > Regards, > Simon > Best regards, Mario
Hi Mario, On 11 April 2018 at 00:35, Mario Six <mario.six@gdsys.cc> wrote: > Hi Simon, > > On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Mario, >> >> On 28 March 2018 at 20:40, Mario Six <mario.six@gdsys.cc> wrote: >>> Add a power_on function to the display uclass to allow devices to be >>> probed and powered-on separately. >> >> Is this different from the 'enable' method? >> > > I was thinking that this is more low-level than the enable method, which also > sets display parameters. I could also use the enable method if it's alright to > disregard the panel_bpp and timing parameters. Yes it's fine to ignore those. I'm just trying to use existing API calls where possible (without being silly about it!) > >> Also note that we have a panel uclass that might be useful. >> > > The Logicore driver is probably more low-level than that, but I'll take a look. OK. Conceptually a panel is just a display panel, so could be of any type, I think. Regards, Simon
Hi Simon, On Thu, Apr 12, 2018 at 6:36 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Mario, > > On 11 April 2018 at 00:35, Mario Six <mario.six@gdsys.cc> wrote: >> Hi Simon, >> >> On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Mario, >>> >>> On 28 March 2018 at 20:40, Mario Six <mario.six@gdsys.cc> wrote: >>>> Add a power_on function to the display uclass to allow devices to be >>>> probed and powered-on separately. >>> >>> Is this different from the 'enable' method? >>> >> >> I was thinking that this is more low-level than the enable method, which also >> sets display parameters. I could also use the enable method if it's alright to >> disregard the panel_bpp and timing parameters. > > Yes it's fine to ignore those. I'm just trying to use existing API > calls where possible (without being silly about it!) > OK, I'll switch to that function then. >> >>> Also note that we have a panel uclass that might be useful. >>> >> >> The Logicore driver is probably more low-level than that, but I'll take a look. > > OK. Conceptually a panel is just a display panel, so could be of any > type, I think. > Took another look: The panel uclass is still a bit thin, but the only operation (enable_backlight) sounds more that it's supposed to contain drivers for "real" panels (i.e. physical display devices). The Logicore TX really just generates a DP signal and does not display it itself (the upcoming board has no display capability at all, actually). So it probably better fits in the display uclass. > Regards, > Simon > Best regards, Mario
Hi Mario, On 18 April 2018 at 02:30, Mario Six <mario.six@gdsys.cc> wrote: > Hi Simon, > > On Thu, Apr 12, 2018 at 6:36 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Mario, >> >> On 11 April 2018 at 00:35, Mario Six <mario.six@gdsys.cc> wrote: >>> Hi Simon, >>> >>> On Fri, Mar 30, 2018 at 10:41 AM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Mario, >>>> >>>> On 28 March 2018 at 20:40, Mario Six <mario.six@gdsys.cc> wrote: >>>>> Add a power_on function to the display uclass to allow devices to be >>>>> probed and powered-on separately. >>>> >>>> Is this different from the 'enable' method? >>>> >>> >>> I was thinking that this is more low-level than the enable method, which also >>> sets display parameters. I could also use the enable method if it's alright to >>> disregard the panel_bpp and timing parameters. >> >> Yes it's fine to ignore those. I'm just trying to use existing API >> calls where possible (without being silly about it!) >> > > OK, I'll switch to that function then. > >>> >>>> Also note that we have a panel uclass that might be useful. >>>> >>> >>> The Logicore driver is probably more low-level than that, but I'll take a look. >> >> OK. Conceptually a panel is just a display panel, so could be of any >> type, I think. >> > > Took another look: The panel uclass is still a bit thin, but the only operation > (enable_backlight) sounds more that it's supposed to contain drivers for "real" > panels (i.e. physical display devices). The Logicore TX really just generates a > DP signal and does not display it itself (the upcoming board has no display > capability at all, actually). So it probably better fits in the display uclass. OK I see. Please be sure to mention this info in your patches. Regards, Simon
diff --git a/drivers/video/display-uclass.c b/drivers/video/display-uclass.c index e752eb07c4..4865057e94 100644 --- a/drivers/video/display-uclass.c +++ b/drivers/video/display-uclass.c @@ -57,6 +57,16 @@ int display_read_timing(struct udevice *dev, struct display_timing *timing) return edid_get_timing(buf, ret, timing, &panel_bits_per_colour); } +int display_power_on(struct udevice *dev, void *data) +{ + struct dm_display_ops *ops = display_get_ops(dev); + + if (!ops || !ops->power_on) + return -ENOSYS; + + return ops->power_on(dev, data); +} + bool display_in_use(struct udevice *dev) { struct display_plat *disp_uc_plat = dev_get_uclass_platdata(dev); diff --git a/include/display.h b/include/display.h index d0a08d4aaa..bb263be246 100644 --- a/include/display.h +++ b/include/display.h @@ -51,6 +51,15 @@ int display_enable(struct udevice *dev, int panel_bpp, */ bool display_in_use(struct udevice *dev); +/** + * display_power_on() - Power on display port device + * + * @dev: Device to power on + * @data: Optional data needed to power on the display correctly + * @return 0 if OK, -ve on error + */ +int display_power_on(struct udevice *dev, void *data); + struct dm_display_ops { /** * read_timing() - Read information directly @@ -81,6 +90,15 @@ struct dm_display_ops { */ int (*enable)(struct udevice *dev, int panel_bpp, const struct display_timing *timing); + + /** + * power_on() - Power on display port device + * + * @dev: Device to power on + * @data: Optional data needed to power on the display correctly + * @return 0 if OK, -ve on error + */ + int (*power_on)(struct udevice *dev, void *data); }; #define display_get_ops(dev) ((struct dm_display_ops *)(dev)->driver->ops)
Add a power_on function to the display uclass to allow devices to be probed and powered-on separately. Signed-off-by: Mario Six <mario.six@gdsys.cc> --- drivers/video/display-uclass.c | 10 ++++++++++ include/display.h | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+)