On Fri, May 09, 2014 at 08:23:01PM +0530, Ajay Kumar wrote:
> implement basic panel controls as a drm_bridge so that
> the existing bridges can make use of it.
>
> The driver assumes that it is the last entity in the bridge chain.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>
> ---
> .../bindings/drm/bridge/bridge_panel.txt | 45 ++++
Can we please stop adding files to this directory? Device tree bindings
are supposed to be OS agnostic, but DRM is specific to Linux and should
not be used when describing hardware.
> diff --git a/Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt b/Documentation/devicetree/bindings/drm/bridge/bridge_panel.txt
[...]
> + -led-en-gpio:
> + eDP panel LED enable GPIO.
> + Indicates which GPIO needs to be powered up as output
> + to enable the backlight.
Since this is used to control a backlight, then this should really be a
separate node to describe the backlight device (and use the
corresponding backlight driver) rather than duplicating a subset of that
functionality.
> + -panel-pre-enable-delay:
> + delay value in ms required for panel_pre_enable process
> + Delay in ms needed for the eDP panel LCD unit to
> + powerup, and delay needed between panel_VCC and
> + video_enable.
What are panel_VCC or video_enable?
> + -panel-enable-delay:
> + delay value in ms required for panel_enable process
> + Delay in ms needed for the eDP panel backlight/LED unit
> + to powerup, and delay needed between video_enable and
> + BL_EN.
Similarily, what does BL_EN stand for?
> + bridge-panel {
> + compatible = "drm-bridge,panel";
Again, drm- doesn't mean anything outside of Linux (and maybe BSD),
therefore shouldn't be used to describe hardware in device tree.
> diff --git a/drivers/gpu/drm/bridge/bridge_panel.c b/drivers/gpu/drm/bridge/bridge_panel.c
[...]
This duplicates much of the functionality that panels should provide. I
think a better solution would be to properly integrate panels with
bridges.
Thierry
Attachment:
pgpAj9llBO1nD.pgp
Description: PGP signature
