linux-kernel - Re: [PATCH] drm/sun4i: tcon: Add dithering support for RGB565/RGB666 LCD panels

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 17 Oct 2017 21:46:59 +0200
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     Jonathan Liu <net147@...il.com>
Cc:     David Airlie <airlied@...ux.ie>, Chen-Yu Tsai <wens@...e.org>,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-sunxi@...glegroups.com
Subject: Re: [PATCH] drm/sun4i: tcon: Add dithering support for RGB565/RGB666
 LCD panels

Hi,

On Tue, Oct 17, 2017 at 10:09:46PM +1100, Jonathan Liu wrote:
> Dithering is supported on TCON channel 0 which is used for LCD panels.

Expanding a bit the commit log would be great. What is dithering, why
is this needed in the first place, what is it trying to fix, etc.

> Signed-off-by: Jonathan Liu <net147@...il.com>
> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 37 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/sun4i/sun4i_tcon.h | 18 +++++++++++++++++-
>  2 files changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 68751b999877..cf313ca858b3 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -12,11 +12,13 @@
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_connector.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_modes.h>
>  #include <drm/drm_of.h>
> +#include <drm/drm_panel.h>
>  
>  #include <uapi/drm/drm_mode.h>
>  
> @@ -168,7 +170,9 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
>  			  struct drm_display_mode *mode)
>  {
>  	unsigned int bp, hsync, vsync;
> +	u32 bus_format = 0;
>  	u8 clk_delay;
> +	struct drm_connector *connector = tcon->panel->connector;

You shouldn't access the connector directly, but instead use the
current state associated to the encoder.

>  	u32 val = 0;
>  
>  	/* Configure the dot clock */
> @@ -230,6 +234,39 @@ void sun4i_tcon0_mode_set(struct sun4i_tcon *tcon,
>  			   SUN4I_TCON0_IO_POL_HSYNC_POSITIVE | SUN4I_TCON0_IO_POL_VSYNC_POSITIVE,
>  			   val);
>  
> +	if (connector->display_info.num_bus_formats)
> +		bus_format = connector->display_info.bus_formats[0];
> +
> +	switch (bus_format) {
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +	case MEDIA_BUS_FMT_RGB666_1X18:

I guess that has more to do with the bit width than the actual media
bus.

I've created helper for both these two in my LVDS patch set, you might
want to rebase on top of it.

> +		/* Enable dithering */
> +		/* FIXME: Undocumented bits */
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED0_REG, 0x11111111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED1_REG, 0x11111111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED2_REG, 0x11111111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED3_REG, 0x11111111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED4_REG, 0x11111111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_SEED5_REG, 0x11111111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_TAB0_REG, 0x01010000);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_TAB1_REG, 0x15151111);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_TAB2_REG, 0x57575555);
> +		regmap_write(tcon->regs, SUN4I_TCON0_FRM_TAB3_REG, 0x7f7f7777);
> +		val = SUN4I_TCON0_FRM_CTL_ENABLE;
> +
> +		if (bus_format == MEDIA_BUS_FMT_RGB565_1X16) {
> +			val |= SUN4I_TCON0_FRM_CTL_MODE_R;
> +			val |= SUN4I_TCON0_FRM_CTL_MODE_B;
> +		}

This can be moved to the first case in your switch, and just let it
fall through. You won't have to do a comparison twice then.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)