public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Thierry Reding <treding@nvidia.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] video: tegra: refuse to bind to disabled dcs
Date: Wed, 20 Apr 2016 14:49:53 +0200	[thread overview]
Message-ID: <20160420124952.GA20346@ulmo.ba.sec> (raw)
In-Reply-To: <1461104370-20439-2-git-send-email-swarren@wwwdotorg.org>

On Tue, Apr 19, 2016 at 04:19:30PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> This prevents the following boot-time message on any board where only the
> first DC is in use, yet the DC's DT node is enabled:
> 
> stdio_add_devices: Video device failed (ret=-22)
> 
> (This happens on at least Harmony, Ventana, and likely any other Tegra20
> board with display enabled other than Seaboard).
> 
> The Tegra DC's DT node represents a display controller. It may itself
> drive an integrated RGB display output, or be used by some other display
> controller such as HDMI. For this reason the DC node itself is not
> enabled/disabled in DT; the DC itself is considered a shared resource, not
> the final (board-specific) display output. The node should instantiate a
> display output driver only if the rgb subnode is enabled. Other output
> drivers are free to use the DC if they are enabled and their DT node
> references the DC's DT node. Adapt the Tegra display drivers' bind()
> routine to only bind to the DC's DT node if the RGB subnode is enabled.
> 
> Now that the display driver does the right thing, remove the workaround
> for this issue from Seaboard's DT file.
> 
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> Thierry, I assume this is how the DC nodes are intended to be interpreted?
> It's certainly how the kernel's DT files and driver work, even if by
> accident.

No, it's entirely intentional. The rationale is, as you already hinted
at in the commit message, that the display controllers don't define an
active display output, but rather display outputs (such as RGB) will
request a display controller for use in building the display pipeline.

RGB is a bit of a special case because it's tied to the respective
display controllers (their registers are part of the DC register space)
whereas the other types of output (HDMI, DSI, eDP, ...) can typically
take pixel data from either of the display controllers.

> ---
>  arch/arm/dts/tegra20-seaboard.dts | 4 ----
>  drivers/video/tegra.c             | 7 +++++++
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/dts/tegra20-seaboard.dts b/arch/arm/dts/tegra20-seaboard.dts
> index eada59073efc..5893d2a3f84e 100644
> --- a/arch/arm/dts/tegra20-seaboard.dts
> +++ b/arch/arm/dts/tegra20-seaboard.dts
> @@ -40,10 +40,6 @@
>  				nvidia,panel = <&lcd_panel>;
>  			};
>  		};
> -
> -		dc at 54240000 {
> -			status = "disabled";
> -		};
>  	};
>  
>  	/* This is not used in U-Boot, but is expected to be in kernel .dts */
> diff --git a/drivers/video/tegra.c b/drivers/video/tegra.c
> index 7fd10e6af35e..c01809e89e14 100644
> --- a/drivers/video/tegra.c
> +++ b/drivers/video/tegra.c
> @@ -620,6 +620,13 @@ static int tegra_lcd_ofdata_to_platdata(struct udevice *dev)
>  static int tegra_lcd_bind(struct udevice *dev)
>  {
>  	struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
> +	const void *blob = gd->fdt_blob;
> +	int node = dev->of_offset;
> +	int rgb;
> +
> +	rgb = fdt_subnode_offset(blob, node, "rgb");
> +	if ((rgb < 0) || !fdtdec_get_is_enabled(blob, rgb))
> +		return -ENODEV;

I'm not intimately familiar with the structure of the display driver in
U-Boot, but it seems like we'd need to conditionalize this depending on
what output the display pipeline uses. I suppose it would be up to its
driver to register a stdio device, while making use of some library code
to configure the display controller. My understanding was from looking
at this a long time ago, that U-Boot only supports RGB output on Tegra20
through Tegra114, and there's a completely separate driver that supports
eDP on Tegra124 (Nyan Big I believe), and presumably that will handle
this entirely differently.

Anyway, this looks like the right fix for RGB outputs (such as on
Seaboard), so:

Acked-by: Thierry Reding <treding@nvidia.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160420/698c26c3/attachment.sig>

  reply	other threads:[~2016-04-20 12:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-19 22:19 [U-Boot] [PATCH 1/2] dm: core: allow drivers to refuse to bind Stephen Warren
2016-04-19 22:19 ` [U-Boot] [PATCH 2/2] video: tegra: refuse to bind to disabled dcs Stephen Warren
2016-04-20 12:49   ` Thierry Reding [this message]
2016-04-20 13:16   ` Simon Glass
2016-05-07 19:03     ` Simon Glass
2016-04-20 14:42 ` [U-Boot] [PATCH 1/2] dm: core: allow drivers to refuse to bind Simon Glass
2016-05-04 18:57 ` Stephen Warren
2016-05-04 19:31   ` Simon Glass
2016-05-04 19:46     ` Stephen Warren
2016-05-04 19:48       ` Simon Glass
2016-05-04 20:02         ` Stephen Warren
2016-05-04 20:09           ` Simon Glass
2016-05-07 19:02             ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160420124952.GA20346@ulmo.ba.sec \
    --to=treding@nvidia.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox