[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <32ab39b4-da1c-7f87-74b9-ec64ebdb8dfc@189.cn>
Date: Mon, 22 May 2023 17:39:04 +0800
From: Sui Jingfeng <15330273260@....cn>
To: WANG Xuerui <kernel@...0n.name>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Sui Jingfeng <suijingfeng@...ngson.cn>,
Li Yi <liyi@...ngson.cn>,
Sumit Semwal <sumit.semwal@...aro.org>,
Christian Koenig <christian.koenig@....com>,
Emil Velikov <emil.l.velikov@...il.com>
Cc: linaro-mm-sig@...ts.linaro.org, loongson-kernel@...ts.loongnix.cn,
Geert Uytterhoeven <geert+renesas@...der.be>,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
Javier Martinez Canillas <javierm@...hat.com>,
Nathan Chancellor <nathan@...nel.org>,
Liu Peibao <liupeibao@...ngson.cn>, linux-media@...r.kernel.org
Subject: Re: [PATCH v14 1/2] drm: add kms driver for loongson display
controller
Hi,
On 2023/5/21 20:21, WANG Xuerui wrote:
>> +#ifndef __LSDC_REGS_H__
>> +#define __LSDC_REGS_H__
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/types.h>
>> +
>> +/*
>> + * PIXEL PLL Reference clock
>> + */
>> +#define LSDC_PLL_REF_CLK 100000 /* kHz */
>
> Consider naming it like "LSDC_PLL_REF_CLK_KHZ" for it to be
> self-documenting?
>
Indeed, this is really reasonable. Can be self-documenting.
thanks.
>> +
>> +/*
>> + * Those PLL registers are relative to LSxxxxx_CFG_REG_BASE. xxxxx =
>> 7A1000,
>> + * 7A2000, 2K2000, 2K1000 etc.
>> + */
>> +
>> +/* LS7A1000 */
>> +
>> +#define LS7A1000_PIXPLL0_REG 0x04B0
>> +#define LS7A1000_PIXPLL1_REG 0x04C0
>> +
>> +/* The DC, GPU, Graphic Memory Controller share the single gfxpll */
>> +#define LS7A1000_PLL_GFX_REG 0x0490
>> +
>> +#define LS7A1000_CONF_REG_BASE 0x10010000
>> +
>> +/* LS7A2000 */
>> +
>> +#define LS7A2000_PIXPLL0_REG 0x04B0
>> +#define LS7A2000_PIXPLL1_REG 0x04C0
>> +
>> +/* The DC, GPU, Graphic Memory Controller share the single gfxpll */
>> +#define LS7A2000_PLL_GFX_REG 0x0490
>> +
>> +#define LS7A2000_CONF_REG_BASE 0x10010000
>> +
>> +/* For LSDC_CRTCx_CFG_REG */
>> +#define CFG_PIX_FMT_MASK GENMASK(2, 0)
>> +
>> +enum lsdc_pixel_format {
>> + LSDC_PF_NONE = 0,
>> + LSDC_PF_XRGB444 = 1, /* [12 bits] */
>> + LSDC_PF_XRGB555 = 2, /* [15 bits] */
>> + LSDC_PF_XRGB565 = 3, /* RGB [16 bits] */
>> + LSDC_PF_XRGB8888 = 4, /* XRGB [32 bits] */
>> +};
>> +
>> +/*
>> + * Each crtc has two set fb address registers usable,
>> FB_REG_IN_USING bit of
>> + * LSDC_CRTCx_CFG_REG indicate which fb address register is in using
>> by the
>> + * CRTC currently. CFG_PAGE_FLIP is used to trigger the switch, the
>> switching
>> + * will be finished at the very next vblank. Trigger it again if you
>> want to
>> + * switch back.
>> + *
>> + * If FB0_ADDR_REG is in using, we write the address to FB0_ADDR_REG,
>> + * if FB1_ADDR_REG is in using, we write the address to FB1_ADDR_REG.
>> + */
>> +#define CFG_PAGE_FLIP BIT(7)
>> +#define CFG_OUTPUT_ENABLE BIT(8)
>> +#define CFG_HW_CLONE BIT(9)
>> +/* Indicate witch fb addr reg is in using, currently. read only */
>> +#define FB_REG_IN_USING BIT(11)
>> +#define CFG_GAMMA_EN BIT(12)
>> +
>> +/* The DC get soft reset if this bit changed from "1" to "0", active
>> low */
>> +#define CFG_RESET_N BIT(20)
>> +/* If this bit is set, it say that the CRTC stop working anymore,
>> anchored. */
>> +#define CRTC_ANCHORED BIT(24)
>> +
>> +/*
>> + * The DMA step of the DC in LS7A2000/LS2K2000 is configurable,
>> + * setting those bits on ls7a1000 platform make no effect.
>> + */
>> +#define CFG_DMA_STEP_MASK GENMASK(17, 16)
>> +#define CFG_DMA_STEP_SHIFT 16
>> +enum lsdc_dma_steps {
>> + LSDC_DMA_STEP_256_BYTES = 0,
>> + LSDC_DMA_STEP_128_BYTES = 1,
>> + LSDC_DMA_STEP_64_BYTES = 2,
>> + LSDC_DMA_STEP_32_BYTES = 3,
>> +};
>> +
>> +#define CFG_VALID_BITS_MASK GENMASK(20, 0)
>> +
>> +/* For LSDC_CRTCx_PANEL_CONF_REG */
>> +#define PHY_CLOCK_POL BIT(9)
>> +#define PHY_CLOCK_EN BIT(8)
>> +#define PHY_DE_POL BIT(1)
>> +#define PHY_DATA_EN BIT(0)
>> +
>> +/* For LSDC_CRTCx_HSYNC_REG */
>> +#define HSYNC_INV BIT(31)
>> +#define HSYNC_EN BIT(30)
>> +#define HSYNC_END_MASK GENMASK(28, 16)
>> +#define HSYNC_END_SHIFT 16
>> +#define HSYNC_START_MASK GENMASK(12, 0)
>> +#define HSYNC_START_SHIFT 0
>> +
>> +/* For LSDC_CRTCx_VSYNC_REG */
>> +#define VSYNC_INV BIT(31)
>> +#define VSYNC_EN BIT(30)
>> +#define VSYNC_END_MASK GENMASK(27, 16)
>> +#define VSYNC_END_SHIFT 16
>> +#define VSYNC_START_MASK GENMASK(11, 0)
>> +#define VSYNC_START_SHIFT 0
>> +
>> +/*********** CRTC0 & DISPLAY PIPE0 ***********/
>> +#define LSDC_CRTC0_CFG_REG 0x1240
>> +#define LSDC_CRTC0_FB0_ADDR_LO_REG 0x1260
>> +#define LSDC_CRTC0_FB0_ADDR_HI_REG 0x15A0
>> +#define LSDC_CRTC0_STRIDE_REG 0x1280
>> +#define LSDC_CRTC0_FB_ORIGIN_REG 0x1300
>> +#define LSDC_CRTC0_PANEL_CONF_REG 0x13C0
>> +#define LSDC_CRTC0_HDISPLAY_REG 0x1400
>> +#define LSDC_CRTC0_HSYNC_REG 0x1420
>> +#define LSDC_CRTC0_VDISPLAY_REG 0x1480
>> +#define LSDC_CRTC0_VSYNC_REG 0x14A0
>> +#define LSDC_CRTC0_GAMMA_INDEX_REG 0x14E0
>> +#define LSDC_CRTC0_GAMMA_DATA_REG 0x1500
>> +#define LSDC_CRTC0_FB1_ADDR_LO_REG 0x1580
>> +#define LSDC_CRTC0_FB1_ADDR_HI_REG 0x15C0
>> +
>> +/*********** CTRC1 & DISPLAY PIPE1 ***********/
>
> "CRTC1"
Indeed, thanks for your sharpen eyes.
I will try to solve all other problems you mentioned at next version.
I don't notice this.
Great thanks.
Powered by blists - more mailing lists