[PATCH v6 01/18] video: tegra20: dc: diverge DC per-SOC
Svyatoslav Ryhel
clamor95 at gmail.com
Fri Apr 19 19:16:28 CEST 2024
пт, 19 квіт. 2024 р. о 19:26 Thierry Reding <thierry.reding at gmail.com> пише:
>
> On Tue Jan 23, 2024 at 6:16 PM CET, Svyatoslav Ryhel wrote:
> [...]
> > diff --git a/arch/arm/include/asm/arch-tegra114/display.h b/arch/arm/include/asm/arch-tegra114/display.h
> > new file mode 100644
> > index 0000000000..9411525799
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-tegra114/display.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * (C) Copyright 2010
> > + * NVIDIA Corporation <www.nvidia.com>
> > + */
> > +
> > +#ifndef __ASM_ARCH_TEGRA_DISPLAY_H
> > +#define __ASM_ARCH_TEGRA_DISPLAY_H
> > +
> > +#include <asm/arch-tegra/dc.h>
> > +
> > +/* This holds information about a window which can be displayed */
> > +struct disp_ctl_win {
> > + enum win_color_depth_id fmt; /* Color depth/format */
> > + unsigned int bpp; /* Bits per pixel */
> > + phys_addr_t phys_addr; /* Physical address in memory */
> > + unsigned int x; /* Horizontal address offset (bytes) */
> > + unsigned int y; /* Veritical address offset (bytes) */
> > + unsigned int w; /* Width of source window */
> > + unsigned int h; /* Height of source window */
> > + unsigned int stride; /* Number of bytes per line */
> > + unsigned int out_x; /* Left edge of output window (col) */
> > + unsigned int out_y; /* Top edge of output window (row) */
> > + unsigned int out_w; /* Width of output window in pixels */
> > + unsigned int out_h; /* Height of output window in pixels */
> > +};
> > +
> > +#endif /*__ASM_ARCH_TEGRA_DISPLAY_H*/
>
> One of the earlier patches in the series gets rid of this per-SoC header
> file in favor of a common one. Did this end up here by mistake? It
> doesn't seem to be used.
>
> > diff --git a/drivers/video/tegra20/tegra-dc.c b/drivers/video/tegra20/tegra-dc.c
> > index f53ad46397..7605e77bc1 100644
> > --- a/drivers/video/tegra20/tegra-dc.c
> > +++ b/drivers/video/tegra20/tegra-dc.c
> > @@ -3,7 +3,6 @@
> > * Copyright (c) 2011 The Chromium OS Authors.
> > */
> >
> > -#include <common.h>
> > #include <backlight.h>
> > #include <dm.h>
> > #include <fdtdec.h>
> > @@ -23,10 +22,15 @@
> > #include <asm/arch/pinmux.h>
> > #include <asm/arch/pwm.h>
> > #include <asm/arch/display.h>
> > -#include <asm/arch-tegra/timer.h>
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> > +/* Holder of Tegra per-SOC DC differences */
> > +struct tegra_dc_soc_info {
> > + bool has_timer;
> > + bool has_rgb;
> > +};
> > +
> > /* Information about the display controller */
> > struct tegra_lcd_priv {
> > int width; /* width in pixels */
> > @@ -35,6 +39,7 @@ struct tegra_lcd_priv {
> > struct display_timing timing;
> > struct udevice *panel;
> > struct dc_ctlr *dc; /* Display controller regmap */
> > + const struct tegra_dc_soc_info *soc;
> > fdt_addr_t frame_buffer; /* Address of frame buffer */
> > unsigned pixel_clock; /* Pixel clock in Hz */
> > int dc_clk[2]; /* Contains clk and its parent */
> > @@ -43,8 +48,8 @@ struct tegra_lcd_priv {
> >
> > enum {
> > /* Maximum LCD size we support */
> > - LCD_MAX_WIDTH = 1920,
> > - LCD_MAX_HEIGHT = 1200,
> > + LCD_MAX_WIDTH = 2560,
> > + LCD_MAX_HEIGHT = 1600,
> > LCD_MAX_LOG2_BPP = VIDEO_BPP16,
> > };
> >
> > @@ -110,9 +115,9 @@ static void update_window(struct tegra_lcd_priv *priv,
> > writel(val, &dc->cmd.state_ctrl);
> > }
> >
> > -static int update_display_mode(struct dc_disp_reg *disp,
> > - struct tegra_lcd_priv *priv)
> > +static int update_display_mode(struct tegra_lcd_priv *priv)
> > {
> > + struct dc_disp_reg *disp = &priv->dc->disp;
> > struct display_timing *dt = &priv->timing;
> > unsigned long val;
> > unsigned long rate;
> > @@ -128,14 +133,16 @@ static int update_display_mode(struct dc_disp_reg *disp,
> > &disp->front_porch);
> > writel(dt->hactive.typ | (dt->vactive.typ << 16), &disp->disp_active);
> >
> > - val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> > - val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> > - writel(val, &disp->data_enable_opt);
> > + if (priv->soc->has_rgb) {
> > + val = DE_SELECT_ACTIVE << DE_SELECT_SHIFT;
> > + val |= DE_CONTROL_NORMAL << DE_CONTROL_SHIFT;
> > + writel(val, &disp->data_enable_opt);
> >
> > - val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> > - val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> > - val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> > - writel(val, &disp->disp_interface_ctrl);
> > + val = DATA_FORMAT_DF1P1C << DATA_FORMAT_SHIFT;
> > + val |= DATA_ALIGNMENT_MSB << DATA_ALIGNMENT_SHIFT;
> > + val |= DATA_ORDER_RED_BLUE << DATA_ORDER_SHIFT;
> > + writel(val, &disp->disp_interface_ctrl);
> > + }
> >
> > /*
> > * The pixel clock divider is in 7.1 format (where the bottom bit
> > @@ -147,7 +154,8 @@ static int update_display_mode(struct dc_disp_reg *disp,
> > div = ((rate * 2 + priv->pixel_clock / 2) / priv->pixel_clock) - 2;
> > debug("Display clock %lu, divider %lu\n", rate, div);
> >
> > - writel(0x00010001, &disp->shift_clk_opt);
> > + if (priv->soc->has_rgb)
> > + writel(0x00010001, &disp->shift_clk_opt);
> >
> > val = PIXEL_CLK_DIVIDER_PCD1 << PIXEL_CLK_DIVIDER_SHIFT;
> > val |= div << SHIFT_CLK_DIVIDER_SHIFT;
> > @@ -174,6 +182,7 @@ static void basic_init(struct dc_cmd_reg *cmd)
> > writel(val, &cmd->disp_pow_ctrl);
> >
> > val = readl(&cmd->disp_cmd);
> > + val &= ~CTRL_MODE_MASK;
> > val |= CTRL_MODE_C_DISPLAY << CTRL_MODE_SHIFT;
>
> This seems unrelated to the rest here, but probably not worth splitting
> it into a separate patch. I vaguely recall that this wasn't really
> necessary because we do the reset prior to initialization and the
> register is all zeroes by default?
>
> > writel(val, &cmd->disp_cmd);
> > }
> > @@ -229,8 +238,8 @@ static void rgb_enable(struct dc_com_reg *com)
> > writel(rgb_sel_tab[i], &com->pin_output_sel[i]);
> > }
> >
> > -static int setup_window(struct disp_ctl_win *win,
> > - struct tegra_lcd_priv *priv)
> > +static int setup_window(struct tegra_lcd_priv *priv,
> > + struct disp_ctl_win *win)
> > {
> > if (priv->rotation) {
> > win->x = priv->width * 2;
> > @@ -274,12 +283,11 @@ static int setup_window(struct disp_ctl_win *win,
> > * You should pass in the U-Boot address here, and check the contents of
> > * struct tegra_lcd_priv to see what was actually chosen.
> > *
> > - * @param blob Device tree blob
> > * @param priv Driver's private data
> > * @param default_lcd_base Default address of LCD frame buffer
> > * Return: 0 if ok, -1 on error (unsupported bits per pixel)
> > */
> > -static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> > +static int tegra_display_probe(struct tegra_lcd_priv *priv,
> > void *default_lcd_base)
> > {
> > struct disp_ctl_win window;
> > @@ -288,7 +296,7 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> > priv->frame_buffer = (u32)default_lcd_base;
> >
> > /*
> > - * We halve the rate if DISP1 paret is PLLD, since actual parent
> > + * We halve the rate if DISP1 parent is PLLD, since actual parent
> > * is plld_out0 which is PLLD divided by 2.
> > */
> > if (priv->dc_clk[1] == CLOCK_ID_DISPLAY)
> > @@ -303,13 +311,17 @@ static int tegra_display_probe(const void *blob, struct tegra_lcd_priv *priv,
> > rate);
> >
> > basic_init(&priv->dc->cmd);
> > - basic_init_timer(&priv->dc->disp);
> > - rgb_enable(&priv->dc->com);
> > +
> > + if (priv->soc->has_timer)
> > + basic_init_timer(&priv->dc->disp);
> > +
> > + if (priv->soc->has_rgb)
> > + rgb_enable(&priv->dc->com);
> >
> > if (priv->pixel_clock)
> > - update_display_mode(&priv->dc->disp, priv);
> > + update_display_mode(priv);
> >
> > - if (setup_window(&window, priv))
> > + if (setup_window(priv, &window))
> > return -1;
> >
> > update_window(priv, &window);
> > @@ -322,7 +334,6 @@ static int tegra_lcd_probe(struct udevice *dev)
> > struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> > struct tegra_lcd_priv *priv = dev_get_priv(dev);
> > - const void *blob = gd->fdt_blob;
> > int ret;
> >
> > /* Initialize the Tegra display controller */
> > @@ -330,8 +341,8 @@ static int tegra_lcd_probe(struct udevice *dev)
> > funcmux_select(PERIPH_ID_DISP1, FUNCMUX_DEFAULT);
> > #endif
> >
> > - if (tegra_display_probe(blob, priv, (void *)plat->base)) {
> > - printf("%s: Failed to probe display driver\n", __func__);
> > + if (tegra_display_probe(priv, (void *)plat->base)) {
> > + debug("%s: Failed to probe display driver\n", __func__);
>
> Shouldn't this remain a printf() to make it more visible in the logs
> what's going on? I guess people will notice anyway when the display
> doesn't turn on, but this is good information to know when
> troubleshooting.
>
debug() is preferred to reduce resulting image size.
> > return -1;
> > }
> >
> > @@ -383,6 +394,8 @@ static int tegra_lcd_of_to_plat(struct udevice *dev)
> > return -EINVAL;
> > }
> >
> > + priv->soc = (struct tegra_dc_soc_info *)dev_get_driver_data(dev);
> > +
> > ret = clock_decode_pair(dev, priv->dc_clk);
> > if (ret < 0) {
> > debug("%s: Cannot decode clocks for '%s' (ret = %d)\n",
> > @@ -464,19 +477,43 @@ static int tegra_lcd_bind(struct udevice *dev)
> > static const struct video_ops tegra_lcd_ops = {
> > };
> >
> > +static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
> > + .has_timer = true,
> > + .has_rgb = true,
> > +};
> > +
> > +static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
> > + .has_timer = false,
> > + .has_rgb = true,
> > +};
> > +
> > +static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
> > + .has_timer = false,
> > + .has_rgb = false,
> > +};
>
> My recollection is that technically Tegra114 still supports RGB, but it
> ends up never being used on any of the platforms that I know of. On
> Linux we base the decision to initialize the corresponding registers on
> the status property of the "rgb" node of each display controller.
> Perhaps that's something that U-Boot could do as well? That would avoid
> programming these registers on Tegra20 and Tegra30 devices that don't
> use RGB.
>
This may be implemented in follow up. Please keep in mind that video subsystem
existing in U-Boot is much simpler then Linux framework and some stuff may not
be easy to implement right away. Thanks.
> Thierry
More information about the U-Boot
mailing list