пт, 19 квіт. 2024 р. о 19:26 Thierry Reding <thierry.red...@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