[PATCH 2/2] drm/tilcdc: Remove unnecessary struct tilcdc_panel_info members
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Mar 6 01:27:43 UTC 2019
Hi Jyri,
Thank you for the patch.
On Fri, Feb 15, 2019 at 10:13:16AM +0200, Jyri Sarha wrote:
> Most of the struct tilcdc_panel_info data members, that are also
> exposed in dts binding, are essentially display IP register bits that
> should not need customization per connected display basis. This patch
> removes them, both from the binding and the struct. The removed data
> members have sane static values and there should be no need to expose
> them in the device tree, certainly not in the panel node.
>
> The removed data members are: ac_bias, ac_bias_intrpt, dma_burst_sz,
> bpp, fdd, sync-ctrl, tft_alt_mode, and raster_order. All of those are
> removed from the driver implementation and from bundled tilcdc panel
> binding. The driver now configures the tilcdc with the sane static
> values, instead of whatever the driver finds from the struct
> tilcdc_panel_info.
>
> The patch does does not cause any functional change to any platform
> supported by Linux mainline kernel and devicetree files.
>
> Signed-off-by: Jyri Sarha <jsarha at ti.com>
> ---
> .../bindings/display/tilcdc/panel.txt | 20 +++++---
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 50 +++++--------------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 27 ----------
> drivers/gpu/drm/tilcdc/tilcdc_external.c | 16 ------
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 9 ----
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 8 ---
> 6 files changed, 24 insertions(+), 106 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/tilcdc/panel.txt b/Documentation/devicetree/bindings/display/tilcdc/panel.txt
> index 808216310ea2..b3d63e3db714 100644
> --- a/Documentation/devicetree/bindings/display/tilcdc/panel.txt
> +++ b/Documentation/devicetree/bindings/display/tilcdc/panel.txt
> @@ -3,15 +3,19 @@ Device-Tree bindings for tilcdc DRM generic panel output driver
> Required properties:
> - compatible: value should be "ti,tilcdc,panel".
> - panel-info: configuration info to configure LCDC correctly for the panel
> - - ac-bias: AC Bias Pin Frequency
> - - ac-bias-intrpt: AC Bias Pin Transitions per Interrupt
> - - dma-burst-sz: DMA burst size
> - - bpp: Bits per pixel
> - - fdd: FIFO DMA Request Delay
> - sync-edge: Horizontal and Vertical Sync Edge: 0=rising 1=falling
> - - sync-ctrl: Horizontal and Vertical Sync: Control: 0=ignore
> - - raster-order: Raster Data Order Select: 1=Most-to-least 0=Least-to-most
> - - fifo-th: DMA FIFO threshold
> +
> + - For compatibility with the older driver versions it is still good
> + keep these obsolete properties (with their recommended vaules):
> + - ac-bias = <255>;
> + - ac-bias-intrpt = <0>;
> + - dma-burst-sz = <16>;
> + - bpp = <16>;
> + - fdd = <0x80>;
> + - sync-ctrl = <1>;
> + - raster-order = <0>;
> + - fifo-th = <0>;
> +
I don't think you need to keep this text, the properties will be
available from the git history. We actually tend to do it the other way
around, and remove properties from the bindings and keep them in the
driver for backward-compatibility purpose. If removal of the properties
don't cause issues (I'll trust you on that) then backward-compatibility
code isn't mandatory.
> - display-timings: typical videomode of lcd panel. Multiple video modes
> can be listed if the panel supports multiple timings, but the 'native-mode'
> should be the preferred/default resolution. Refer to
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 1067e702c22c..ea2e4afad75c 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -295,29 +295,11 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> return;
>
> /* Configure the Burst Size and fifo threshold of DMA: */
> - reg = tilcdc_read(dev, LCDC_DMA_CTRL_REG) & ~0x00000770;
> - switch (info->dma_burst_sz) {
> - case 1:
> - reg |= LCDC_DMA_BURST_SIZE(LCDC_DMA_BURST_1);
> - break;
> - case 2:
> - reg |= LCDC_DMA_BURST_SIZE(LCDC_DMA_BURST_2);
> - break;
> - case 4:
> - reg |= LCDC_DMA_BURST_SIZE(LCDC_DMA_BURST_4);
> - break;
> - case 8:
> - reg |= LCDC_DMA_BURST_SIZE(LCDC_DMA_BURST_8);
> - break;
> - case 16:
> - reg |= LCDC_DMA_BURST_SIZE(LCDC_DMA_BURST_16);
> - break;
> - default:
> - dev_err(dev->dev, "invalid burst size\n");
> - return;
> - }
> - reg |= (info->fifo_th << 8);
> - tilcdc_write(dev, LCDC_DMA_CTRL_REG, reg);
> + tilcdc_write_mask(dev, LCDC_DMA_CTRL_REG,
> + LCDC_DMA_FIFO_THRESHOLD(0) |
> + LCDC_DMA_BURST_SIZE(LCDC_DMA_BURST_16),
> + LCDC_DMA_FIFO_THRESHOLD_MASK |
> + LCDC_DMA_BURST_SIZE_MASK);
>
> /* Configure timings: */
> hbp = mode->htotal - mode->hsync_end;
> @@ -331,9 +313,11 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> mode->hdisplay, mode->vdisplay, hbp, hfp, hsw, vbp, vfp, vsw);
>
> /* Set AC Bias Period and Number of Transitions per Interrupt: */
> - reg = tilcdc_read(dev, LCDC_RASTER_TIMING_2_REG) & ~0x000fff00;
> - reg |= LCDC_AC_BIAS_FREQUENCY(info->ac_bias) |
> - LCDC_AC_BIAS_TRANSITIONS_PER_INT(info->ac_bias_intrpt);
> + reg = tilcdc_read(dev, LCDC_RASTER_TIMING_2_REG);
> + reg &= ~LCDC_AC_BIAS_FREQUENCY_MASK;
> + reg |= LCDC_AC_BIAS_FREQUENCY(255);
> + reg &= ~LCDC_AC_BIAS_TRANSITIONS_PER_INT_MASK;
> + reg |= LCDC_AC_BIAS_TRANSITIONS_PER_INT(0);
>
> /*
> * subtract one from hfp, hbp, hsw because the hardware uses
> @@ -383,8 +367,6 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> LCDC_V2_TFT_24BPP_MODE | LCDC_V2_TFT_24BPP_UNPACK |
> 0x000ff000 /* Palette Loading Delay bits */);
> reg |= LCDC_TFT_MODE; /* no monochrome/passive support */
> - if (info->tft_alt_mode)
> - reg |= LCDC_TFT_ALT_ENABLE;
> if (priv->rev == 2) {
> switch (fb->format->format) {
> case DRM_FORMAT_BGR565:
> @@ -403,7 +385,6 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> return;
> }
> }
> - reg |= info->fdd < 12;
> tilcdc_write(dev, LCDC_RASTER_CTRL_REG, reg);
>
> if (info->invert_pxl_clk)
> @@ -411,10 +392,8 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> else
> tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_INVERT_PIXEL_CLOCK);
>
> - if (info->sync_ctrl)
> - tilcdc_set(dev, LCDC_RASTER_TIMING_2_REG, LCDC_SYNC_CTRL);
> - else
> - tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_SYNC_CTRL);
> + /* Take sync polarity from LCDC_SYNC_EDGE bit */
> + tilcdc_set(dev, LCDC_RASTER_TIMING_2_REG, LCDC_SYNC_CTRL);
>
> if (info->sync_edge)
> tilcdc_set(dev, LCDC_RASTER_TIMING_2_REG, LCDC_SYNC_EDGE);
> @@ -431,11 +410,6 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
> else
> tilcdc_clear(dev, LCDC_RASTER_TIMING_2_REG, LCDC_INVERT_VSYNC);
>
> - if (info->raster_order)
> - tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ORDER);
> - else
> - tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ORDER);
> -
> tilcdc_crtc_set_clk(crtc);
>
> tilcdc_crtc_load_palette(crtc);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index 62cea5ff5558..acf27e22c409 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -125,38 +125,11 @@ void tilcdc_module_cleanup(struct tilcdc_module *mod);
> */
> struct tilcdc_panel_info {
>
> - /* AC Bias Pin Frequency */
> - uint32_t ac_bias;
> -
> - /* AC Bias Pin Transitions per Interrupt */
> - uint32_t ac_bias_intrpt;
> -
> - /* DMA burst size */
> - uint32_t dma_burst_sz;
> -
> - /* Bits per pixel */
> - uint32_t bpp;
> -
> - /* FIFO DMA Request Delay */
> - uint32_t fdd;
> -
> - /* TFT Alternative Signal Mapping (Only for active) */
> - bool tft_alt_mode;
> -
> /* Invert pixel clock */
> bool invert_pxl_clk;
>
> /* Horizontal and Vertical Sync Edge: 0=rising 1=falling */
> uint32_t sync_edge;
> -
> - /* Horizontal and Vertical Sync: Control: 0=ignore */
> - uint32_t sync_ctrl;
> -
> - /* Raster Data Order Select: 1=Most-to-least 0=Least-to-most */
> - uint32_t raster_order;
> -
> - /* DMA FIFO threshold */
> - uint32_t fifo_th;
> };
>
> #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index b4eaf9bc87f8..9626cccf0d17 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -16,28 +16,12 @@
> #include "tilcdc_external.h"
>
> static const struct tilcdc_panel_info panel_info_tda998x = {
> - .ac_bias = 255,
> - .ac_bias_intrpt = 0,
> - .dma_burst_sz = 16,
> - .bpp = 16,
> - .fdd = 0x80,
> - .tft_alt_mode = 0,
> .invert_pxl_clk = 1,
> .sync_edge = 1,
> - .sync_ctrl = 1,
> - .raster_order = 0,
> };
>
> static const struct tilcdc_panel_info panel_info_default = {
> - .ac_bias = 255,
> - .ac_bias_intrpt = 0,
> - .dma_burst_sz = 16,
> - .bpp = 16,
> - .fdd = 0x80,
> - .tft_alt_mode = 0,
> .sync_edge = 0,
> - .sync_ctrl = 1,
> - .raster_order = 0,
> };
>
> static int tilcdc_external_mode_valid(struct drm_connector *connector,
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> index a1acab39d87f..1f788171cdbb 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> @@ -292,18 +292,9 @@ static struct tilcdc_panel_info *of_get_panel_info(struct device_node *np)
> if (!info)
> goto put_node;
>
> - ret |= of_property_read_u32(info_np, "ac-bias", &info->ac_bias);
> - ret |= of_property_read_u32(info_np, "ac-bias-intrpt", &info->ac_bias_intrpt);
> - ret |= of_property_read_u32(info_np, "dma-burst-sz", &info->dma_burst_sz);
> - ret |= of_property_read_u32(info_np, "bpp", &info->bpp);
> - ret |= of_property_read_u32(info_np, "fdd", &info->fdd);
> ret |= of_property_read_u32(info_np, "sync-edge", &info->sync_edge);
> - ret |= of_property_read_u32(info_np, "sync-ctrl", &info->sync_ctrl);
> - ret |= of_property_read_u32(info_np, "raster-order", &info->raster_order);
> - ret |= of_property_read_u32(info_np, "fifo-th", &info->fifo_th);
>
> /* optional: */
> - info->tft_alt_mode = of_property_read_bool(info_np, "tft-alt-mode");
> info->invert_pxl_clk = of_property_read_bool(info_np, "invert-pxl-clk");
>
> if (ret) {
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index daebf1aa6b0a..12404401b337 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -34,15 +34,7 @@ struct tfp410_module {
>
>
> static const struct tilcdc_panel_info dvi_info = {
> - .ac_bias = 255,
> - .ac_bias_intrpt = 0,
> - .dma_burst_sz = 16,
> - .bpp = 16,
> - .fdd = 0x80,
> - .tft_alt_mode = 0,
> .sync_edge = 0,
> - .sync_ctrl = 1,
> - .raster_order = 0,
> };
>
> /*
--
Regards,
Laurent Pinchart
More information about the dri-devel
mailing list