linux-kernel - Re: [RFC 0/4] Use the Common Display Framework in tegra-drm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 30 Jan 2013 08:40:20 +0100
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Alexandre Courbot <acourbot@...dia.com>
Cc:	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Stephen Warren <swarren@...dotorg.org>,
	Mark Zhang <markz@...dia.com>, linux-kernel@...r.kernel.org,
	linux-fbdev@...r.kernel.org, linux-tegra@...r.kernel.org,
	gnurou@...il.com
Subject: Re: [RFC 0/4] Use the Common Display Framework in tegra-drm

On Wed, Jan 30, 2013 at 12:02:15PM +0900, Alexandre Courbot wrote:
> This series leverages the (still work-in-progress) Common Display Framework to
> add panel support to the tegra-drm driver. It also adds a driver for the
> CLAA101WA01A panel used on the Ventana board.
> 
> The CDF is a moving target but Tegra needs some sort of display framework and
> even in its current state the CDF seems to be the best candidate. Besides, by
> using the CDF from now on we hope to provide useful feedback to Laurent and the
> other CDF designers.
> 
> The changes to tegra-drm are rather minimal. Panels are referenced from Tegra DC
> output nodes through the "nvidia,panel" property. This property is looked up
> when a display connect notification is received in order to see if it points to
> the connected display entity. If it does, the entity is then used for output.
> 
> The DPMS states are then propagated to the output entity, which is then supposed
> to call back into the set_stream() hook in order to enable/disable the output
> stream as needed.

Thanks *a lot* for taking care of this Alexandre! From a quick look at
the patches they seem generally fine. I'll go over them in a bit more
detail though.

> Although the overall design seems to work ok, a few specific issues need to be
> addressed:
> 
> 1) The CDF has a get_modes() hook, but this is already implemented by
> tegra_connector_get_modes(). Ideally everything should be moved to the CDF hook,
> but Tegra's implementation uses DRM functions to retrieve the EDID and CDF
> should, AFAIK, remain DRM-agnostic.

Maybe a good option would be to just not implement get_modes() if the
same information can be retrieved via EDID. That is, the DRM driver
could just go and fetch EDID when the nvidia,ddc-i2c-bus is available
(or parse the nvidia,edid blob) and only rely on CDF otherwise.

So for Ventana the only reason why we need CDF is basically the power
sequencing, right?

> 2) There is currently no panel/backlight interaction, e.g. backlight status is
> controlled through FB events, independantly from the panel state. It could make
> sense to have the panel DT node reference the backlight and control it as part
> of its own power on/off sequence. Right now however, a backlight device cannot
> ignore FB events.

I definitely think that we should aim for correct panel and backlight
interaction. Perhaps this could work by looking up the real backlight
via it's phandle and have the CDF driver use the backlight API to
disable or enable the backlight as part of the power sequencing.

I'm not sure what you mean by "cannot ignore FB events"? Can you provide
a concrete problematic use-case?

> 3) Probably related to 2), now the backlight's power controls are part of the
> panel driver, because the pwm-backlight driver cannot control the power
> regulator and enable GPIO. This means that the backlight power is not turned off
> when its brightness is set to 0 through sysfs. Once again this speaks in favor
> of having stronger panel/backlight interaction: for instance, the panel driver
> could reference the backlight and hijack its update_status() op to replace it by
> one that does the correct power sequencing before calling the original function.
> This would require some extra infrastructure though. Another possibility would
> be to have a dedicated backlight driver for each panel, with its own
> "compatible" string.

Hijacking .update_status() sounds a bit risky. But perhaps you could
wrap the real backlight in a CDF backlight to receive notifications.
Obviously you'd get two backlight devices in sysfs, but that turn out
not to be a problem.

Thierry

Content of type "application/pgp-signature" skipped