| 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
| ||
|
Message-ID: <4BBEAB3E.5090709@gmx.de>
Date: Fri, 09 Apr 2010 06:21:18 +0200
From: Florian Tobias Schandinat <FlorianSchandinat@....de>
To: Jonathan Corbet <corbet@....net>
CC: linux-kernel@...r.kernel.org, Harald Welte <laforge@...monks.org>,
JosephChan@....com.tw, ScottFang@...tech.com.cn,
Deepak Saxena <dsaxena@...top.org>,
linux-fbdev-devel@...ts.sourceforge.net
Subject: Re: [PATCH 06/16] viafb: complete support for VX800/VX855 accelerated
framebuffer
Jonathan Corbet schrieb:
> This patch is a painful merge of change
> a90bab567ece3e915d0ccd55ab00c9bb333fa8c0 (viafb: Add support for 2D
> accelerated framebuffer on VX800/VX855) in the OLPC tree, originally by
> Harald Welte. Harald's changelog read:
>
> The VX800/VX820 and the VX855/VX875 chipsets have a different 2D
> acceleration engine called "M1". The M1 engine has some subtle
> (and some not-so-subtle) differences to the previous engines, so
> support for accelerated framebuffer on those chipsets was disabled
> so far.
>
> This merge tries to preserve Harald's changes in the framework of the
> much-changed 2.6.34 viafb code.
>
> Signed-off-by: Jonathan Corbet <corbet@....net>
The patch itself looks good. However I have a few minor issues below
which could be addressed by follow-ups (or ignored for the moment).
> ---
> drivers/video/via/accel.c | 39 +++++++++++++++++++++++++++++++++------
> drivers/video/via/accel.h | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
> index 78c0a2b..7fe1c1d 100644
> --- a/drivers/video/via/accel.c
> +++ b/drivers/video/via/accel.c
> @@ -328,6 +328,7 @@ int viafb_init_engine(struct fb_info *info)
> {
> struct viafb_par *viapar = info->par;
> void __iomem *engine;
> + int highest_reg, i;
> u32 vq_start_addr, vq_end_addr, vq_start_low, vq_end_low, vq_high,
> vq_len, chip_name = viapar->shared->chip_info.gfx_chip_name;
>
> @@ -339,6 +340,18 @@ int viafb_init_engine(struct fb_info *info)
> return -ENOMEM;
> }
>
> + /* Initialize registers to reset the 2D engine */
> + switch (viapar->shared->chip_info.twod_engine) {
> + case VIA_2D_ENG_M1:
> + highest_reg = 0x5c;
> + break;
> + default:
> + highest_reg = 0x40;
> + break;
> + }
> + for (i = 0; i <= highest_reg; i+= 4)
> + writel(0x0, engine + i);
> +
this obsoletes
/* Init 2D engine reg to reset 2D engine */
writel(0x0, engine + VIA_REG_KEYCONTROL);
as VIA_REG_KEYCONTROL is 0x02C.
> switch (chip_name) {
> case UNICHROME_CLE266:
> case UNICHROME_K400:
> @@ -375,6 +388,8 @@ int viafb_init_engine(struct fb_info *info)
> switch (chip_name) {
> case UNICHROME_K8M890:
> case UNICHROME_P4M900:
> + case UNICHROME_VX800:
> + case UNICHROME_VX855:
> writel(0x00100000, engine + VIA_REG_CR_TRANSET);
> writel(0x680A0000, engine + VIA_REG_CR_TRANSPACE);
> writel(0x02000000, engine + VIA_REG_CR_TRANSPACE);
> @@ -409,6 +424,8 @@ int viafb_init_engine(struct fb_info *info)
> switch (chip_name) {
> case UNICHROME_K8M890:
> case UNICHROME_P4M900:
> + case UNICHROME_VX800:
> + case UNICHROME_VX855:
> vq_start_low |= 0x20000000;
> vq_end_low |= 0x20000000;
> vq_high |= 0x20000000;
> @@ -486,15 +503,25 @@ void viafb_wait_engine_idle(struct fb_info *info)
> {
> struct viafb_par *viapar = info->par;
> int loop = 0;
> + u32 mask;
>
> - while (!(readl(viapar->shared->engine_mmio + VIA_REG_STATUS) &
> - VIA_VR_QUEUE_BUSY) && (loop < MAXLOOP)) {
> - loop++;
> - cpu_relax();
> + switch (viapar->shared->chip_info.twod_engine) {
> + case VIA_2D_ENG_H5:
> + case VIA_2D_ENG_M1:
> + mask = VIA_CMD_RGTR_BUSY_M1 | VIA_2D_ENG_BUSY_M1 |
> + VIA_3D_ENG_BUSY_M1;
> + break;
> + default:
As in the previous patch I'd appreciate a default of
printk(KERN_ALERT "viafb_xy: FIXME");
I have to admit to be consistent a few switches that are already there
would need to be changed but that's another issue.
> + while (!(readl(viapar->shared->engine_mmio + VIA_REG_STATUS) &
> + VIA_VR_QUEUE_BUSY) && (loop < MAXLOOP)) {
> + loop++;
> + cpu_relax();
> + }
> + mask = VIA_CMD_RGTR_BUSY | VIA_2D_ENG_BUSY | VIA_3D_ENG_BUSY;
> + break;
> }
>
> - while ((readl(viapar->shared->engine_mmio + VIA_REG_STATUS) &
> - (VIA_CMD_RGTR_BUSY | VIA_2D_ENG_BUSY | VIA_3D_ENG_BUSY)) &&
> + while ((readl(viapar->shared->engine_mmio + VIA_REG_STATUS) & mask) &&
> (loop < MAXLOOP)) {
> loop++;
> cpu_relax();
> diff --git a/drivers/video/via/accel.h b/drivers/video/via/accel.h
> index 615c84a..2c122d2 100644
> --- a/drivers/video/via/accel.h
> +++ b/drivers/video/via/accel.h
> @@ -67,6 +67,34 @@
> /* from 0x100 to 0x1ff */
> #define VIA_REG_COLORPAT 0x100
>
> +/* defines for VIA 2D registers for vt3353/3409 (M1 engine)*/
> +#define VIA_REG_GECMD_M1 0x000
> +#define VIA_REG_GEMODE_M1 0x004
> +#define VIA_REG_GESTATUS_M1 0x004 /* as same as VIA_REG_GEMODE */
> +#define VIA_REG_PITCH_M1 0x008 /* pitch of src and dst */
> +#define VIA_REG_DIMENSION_M1 0x00C /* width and height */
> +#define VIA_REG_DSTPOS_M1 0x010
> +#define VIA_REG_LINE_XY_M1 0x010
> +#define VIA_REG_DSTBASE_M1 0x014
> +#define VIA_REG_SRCPOS_M1 0x018
> +#define VIA_REG_LINE_K1K2_M1 0x018
> +#define VIA_REG_SRCBASE_M1 0x01C
> +#define VIA_REG_PATADDR_M1 0x020
> +#define VIA_REG_MONOPAT0_M1 0x024
> +#define VIA_REG_MONOPAT1_M1 0x028
> +#define VIA_REG_OFFSET_M1 0x02C
> +#define VIA_REG_LINE_ERROR_M1 0x02C
> +#define VIA_REG_CLIPTL_M1 0x040 /* top and left of clipping */
> +#define VIA_REG_CLIPBR_M1 0x044 /* bottom and right of clipping */
> +#define VIA_REG_KEYCONTROL_M1 0x048 /* color key control */
> +#define VIA_REG_FGCOLOR_M1 0x04C
> +#define VIA_REG_DSTCOLORKEY_M1 0x04C /* as same as VIA_REG_FG */
> +#define VIA_REG_BGCOLOR_M1 0x050
> +#define VIA_REG_SRCCOLORKEY_M1 0x050 /* as same as VIA_REG_BG */
> +#define VIA_REG_MONOPATFGC_M1 0x058 /* Add BG color of Pattern. */
> +#define VIA_REG_MONOPATBGC_M1 0x05C /* Add FG color of Pattern. */
> +#define VIA_REG_COLORPAT_M1 0x100 /* from 0x100 to 0x1ff */
> +
All of these defines are unused. I admit that it is my fault. I've not
yet found a way I like to use them as the meaning of the same hardware
address changed. I think the most clean long term solution would be to
put the engines in separate files and the definitions in private headers
to at least avoid the need to decode the engine version in the name.
However as the old headers already contain a bunch of trash feel free to
ignore this issue and add it for now.
> /* VIA_REG_PITCH(0x38): Pitch Setting */
> #define VIA_PITCH_ENABLE 0x80000000
>
> @@ -157,6 +185,18 @@
> /* Virtual Queue is busy */
> #define VIA_VR_QUEUE_BUSY 0x00020000
>
> +/* VIA_REG_STATUS(0x400): Engine Status for H5 */
> +#define VIA_CMD_RGTR_BUSY_H5 0x00000010 /* Command Regulator is busy */
> +#define VIA_2D_ENG_BUSY_H5 0x00000002 /* 2D Engine is busy */
> +#define VIA_3D_ENG_BUSY_H5 0x00001FE1 /* 3D Engine is busy */
> +#define VIA_VR_QUEUE_BUSY_H5 0x00000004 /* Virtual Queue is busy */
> +
> +/* VIA_REG_STATUS(0x400): Engine Status for VT3353/3409 */
> +#define VIA_CMD_RGTR_BUSY_M1 0x00000010 /* Command Regulator is busy */
> +#define VIA_2D_ENG_BUSY_M1 0x00000002 /* 2D Engine is busy */
> +#define VIA_3D_ENG_BUSY_M1 0x00001FE1 /* 3D Engine is busy */
> +#define VIA_VR_QUEUE_BUSY_M1 0x00000004 /* Virtual Queue is busy */
> +
> #define MAXLOOP 0xFFFFFF
>
> #define VIA_BITBLT_COLOR 1
Thanks,
Florian Tobias Schandinat
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists