The following patches are the beginning of an effort to move work done for
the OLPC XO 1.5 machine into the mainline. What's here is basic support
for the VX855 chipset, I2C, suspend/resume, and the OLPC panel display.
What's coming in the future is a reworking of the viafb driver into
something resembling a proper multifunction device, GPIO support, and
camera support. But I'd like to start here.
If there's no objections, I'll start a tree and get it into linux-next in
the near future, with an eye toward a 2.6.35 merge.
Thanks,
jon
Chris Ball (2):
viafb: Add 1200x900 DCON/LCD panel modes for OLPC XO-1.5
viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5
Deepak Saxena (3):
Minimal support for viafb suspend/resume
VIAFB: Update suspend/resume to selectively restore registers
Remove cursor restore hack in viafb
Harald Welte (4):
[FB] viafb: Fix various resource leaks during module_init()
viafb: use proper pci config API
viafb: Determine type of 2D engine and store it in chip_info
viafb: rework the I2C support in the VIA framebuffer driver
Jonathan Corbet (5):
viafb: Unmap the frame buffer on initialization error
viafb: Retain GEMODE reserved bits
viafb: complete support for VX800/VX855 accelerated framebuffer
viafb: rework suspend/resume
viafb: Only suspend/resume on VX855
Paul Fox (2):
suppress verbose debug messages: change printk() to DEBUG_MSG()
fix register save count, so it matches the restore count.
drivers/video/via/accel.c | 87 ++++++++++++++-----
drivers/video/via/accel.h | 40 +++++++++
drivers/video/via/chip.h | 8 +
drivers/video/via/dvi.c | 35 +++----
drivers/video/via/hw.c | 84 +++++++++++++-----
drivers/video/via/hw.h | 4
drivers/video/via/ioctl.h | 2
drivers/video/via/lcd.c | 29 ++++--
drivers/video/via/lcd.h | 2
drivers/video/via/share.h | 8 +
drivers/video/via/via_i2c.c | 172 ++++++++++++++++++++++++--------------
drivers/video/via/via_i2c.h | 43 ++++++---
drivers/video/via/viafbdev.c | 191 +++++++++++++++++++++++++++++++++++++++----
drivers/video/via/viafbdev.h | 5 -
drivers/video/via/viamode.c | 14 +++
drivers/video/via/vt1636.c | 36 +++-----
drivers/video/via/vt1636.h | 2
17 files changed, 568 insertions(+), 194 deletions(-)
This was part of Harald's "make viafb a first-class citizen using
pci_driver" patch, but somehow got dropped when that patch went into
mainline.
Signed-off-by: Jonathan Corbet <[email protected]>
---
drivers/video/via/viafbdev.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 91bfe6d..4a34d4f 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1991,7 +1991,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
if (!viafbinfo1) {
printk(KERN_ERR
"allocate the second framebuffer struct error\n");
- goto out_delete_i2c;
+ goto out_unmap_screen;
}
viaparinfo1 = viafbinfo1->par;
memcpy(viaparinfo1, viaparinfo, viafb_par_length);
@@ -2086,6 +2086,8 @@ out_dealloc_cmap:
out_fb1_release:
if (viafbinfo1)
framebuffer_release(viafbinfo1);
+out_unmap_screen:
+ iounmap(viafbinfo->screen_base);
out_delete_i2c:
viafb_delete_i2c_buss(viaparinfo);
out_fb_release:
--
1.7.0.1
From: Harald Welte <[email protected]>
This will help us for the upcoming support for 2D acceleration using
the M1 engine.
[jc: fixed merge conflicts]
Signed-off-by: Harald Welte <[email protected]>
---
drivers/video/via/chip.h | 8 ++++++++
drivers/video/via/hw.c | 15 +++++++++++++++
2 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/drivers/video/via/chip.h b/drivers/video/via/chip.h
index 474f428..f799e2c 100644
--- a/drivers/video/via/chip.h
+++ b/drivers/video/via/chip.h
@@ -122,9 +122,17 @@ struct lvds_chip_information {
int i2c_port;
};
+/* The type of 2D engine */
+enum via_2d_engine {
+ VIA_2D_ENG_H2,
+ VIA_2D_ENG_H5,
+ VIA_2D_ENG_M1,
+};
+
struct chip_information {
int gfx_chip_name;
int gfx_chip_revision;
+ enum via_2d_engine twod_engine;
struct tmds_chip_information tmds_chip_info;
struct lvds_chip_information lvds_chip_info;
struct lvds_chip_information lvds_chip_info2;
diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index f2e4bda..963704e 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -2034,6 +2034,21 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
CX700_REVISION_700;
}
}
+
+ /* Determine which 2D engine we have */
+ switch (viaparinfo->chip_info->gfx_chip_name) {
+ case UNICHROME_VX800:
+ case UNICHROME_VX855:
+ viaparinfo->chip_info->twod_engine = VIA_2D_ENG_M1;
+ break;
+ case UNICHROME_K8M890:
+ case UNICHROME_P4M900:
+ viaparinfo->chip_info->twod_engine = VIA_2D_ENG_H5;
+ break;
+ default:
+ viaparinfo->chip_info->twod_engine = VIA_2D_ENG_H2;
+ break;
+ }
}
static void init_tmds_chip_info(void)
--
1.7.0.1
From: Deepak Saxena <[email protected]>
The existing code in VIAFB, taken from the openchrome driver,
causes the system to lock up upon moving the mouse pointer. This
patch uses the selective register restore from the official
VIA driver to keep this from happening.
This fixes http://dev.laptop.org/ticket/9565
[jc: extensive reworking to merge with 2.6.34 and to deal with some
coding issues. This code still needs improvement, though; that will
come in a later patch.]
Signed-off-by: Deepak Saxena <[email protected]>
---
drivers/video/via/viafbdev.c | 79 +++++++++++++++++++++++++++++++++++++++---
drivers/video/via/viafbdev.h | 76 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 148 insertions(+), 7 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 5a5964f..56d5416 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1850,9 +1850,9 @@ static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
if (state.event == PM_EVENT_SUSPEND) {
acquire_console_sem();
- memcpy_fromio(viaparinfo->shared->saved_regs,
+ memcpy_fromio(&viaparinfo->shared->saved_video_regs,
viaparinfo->shared->engine_mmio + 0x100,
- 0x100 * sizeof(u32));
+ sizeof(struct via_video_regs));
fb_set_suspend(viafbinfo, 1);
@@ -1869,6 +1869,10 @@ static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
static int viafb_resume(struct pci_dev *pdev)
{
+ volatile struct via_video_regs *viaVidEng =
+ (volatile struct via_video_regs *)(viaparinfo->shared->engine_mmio + 0x200);
+ struct via_video_regs *localVidEng = &viaparinfo->shared->saved_video_regs;
+
acquire_console_sem();
pci_set_power_state(pdev, PCI_D0);
pci_restore_state(pdev);
@@ -1876,9 +1880,74 @@ static int viafb_resume(struct pci_dev *pdev)
goto fail;
pci_set_master(pdev);
- memcpy_toio(viaparinfo->shared->engine_mmio + 0x100,
- viaparinfo->shared->saved_regs,
- 0x100 * sizeof(u32));
+ /*
+ * Following set of register restores is black magic takend
+ * from the VIA X driver. Most of it is from the LeaveVT() and
+ * EnterVT() path and some is gleaned by looking at other bits
+ * of code to figure out what registers to touch.
+ */
+ viaVidEng->alphawin_hvstart = localVidEng->alphawin_hvstart;
+ viaVidEng->alphawin_size = localVidEng->alphawin_size;
+ viaVidEng->alphawin_ctl = localVidEng->alphawin_ctl;
+ viaVidEng->alphafb_stride = localVidEng->alphafb_stride;
+ viaVidEng->color_key = localVidEng->color_key;
+ viaVidEng->alphafb_addr = localVidEng->alphafb_addr;
+ viaVidEng->chroma_low = localVidEng->chroma_low;
+ viaVidEng->chroma_up = localVidEng->chroma_up;
+
+ /*VT3314 only has V3*/
+ viaVidEng->video1_ctl = localVidEng->video1_ctl;
+ viaVidEng->video1_fetch = localVidEng->video1_fetch;
+ viaVidEng->video1y_addr1 = localVidEng->video1y_addr1;
+ viaVidEng->video1_stride = localVidEng->video1_stride;
+ viaVidEng->video1_hvstart = localVidEng->video1_hvstart;
+ viaVidEng->video1_size = localVidEng->video1_size;
+ viaVidEng->video1y_addr2 = localVidEng->video1y_addr2;
+ viaVidEng->video1_zoom = localVidEng->video1_zoom;
+ viaVidEng->video1_mictl = localVidEng->video1_mictl;
+ viaVidEng->video1y_addr0 = localVidEng->video1y_addr0;
+ viaVidEng->video1_fifo = localVidEng->video1_fifo;
+ viaVidEng->video1y_addr3 = localVidEng->video1y_addr3;
+ viaVidEng->v1_source_w_h = localVidEng->v1_source_w_h ;
+ viaVidEng->video1_CSC1 = localVidEng->video1_CSC1;
+ viaVidEng->video1_CSC2 = localVidEng->video1_CSC2;
+
+ viaVidEng->snd_color_key = localVidEng->snd_color_key;
+ viaVidEng->v3alpha_prefifo = localVidEng->v3alpha_prefifo;
+ viaVidEng->v3alpha_fifo = localVidEng->v3alpha_fifo;
+ viaVidEng->video3_CSC2 = localVidEng->video3_CSC2;
+ viaVidEng->video3_CSC2 = localVidEng->video3_CSC2;
+ viaVidEng->v3_source_width = localVidEng->v3_source_width;
+ viaVidEng->video3_ctl = localVidEng->video3_ctl;
+ viaVidEng->video3_addr0 = localVidEng->video3_addr0;
+ viaVidEng->video3_addr1 = localVidEng->video3_addr1;
+ viaVidEng->video3_stride = localVidEng->video3_stride;
+ viaVidEng->video3_hvstart = localVidEng->video3_hvstart;
+ viaVidEng->video3_size = localVidEng->video3_size;
+ viaVidEng->v3alpha_fetch = localVidEng->v3alpha_fetch;
+ viaVidEng->video3_zoom = localVidEng->video3_zoom;
+ viaVidEng->video3_mictl = localVidEng->video3_mictl;
+ viaVidEng->video3_CSC1 = localVidEng->video3_CSC1;
+ viaVidEng->video3_CSC2 = localVidEng->video3_CSC2;
+ viaVidEng->compose = localVidEng->compose;
+
+ /*
+ * This _might_ not be needed, likely text mode cursor
+ */
+ viaVidEng->cursor_mode = localVidEng->cursor_mode;
+ viaVidEng->cursor_pos = localVidEng->cursor_pos;
+ viaVidEng->cursor_org = localVidEng->cursor_org;
+ viaVidEng->cursor_bg = localVidEng->cursor_bg;
+ viaVidEng->cursor_fg = localVidEng->cursor_fg;
+
+ /*
+ * Magic register dance to make cursor reappear.
+ *
+ * Currently hardcoded settings need to clean this all
+ * up later...
+ */
+ viaVidEng->hi_control = 0xb6000005;
+ viaVidEng->compose |= 0xc0000000;
fb_set_suspend(viafbinfo, 0);
diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
index 46d41af..23b4afd 100644
--- a/drivers/video/via/viafbdev.h
+++ b/drivers/video/via/viafbdev.h
@@ -39,6 +39,77 @@
#define VIAFB_NUM_I2C 5
+/*
+ * From VIA X Driver; these are the registers we save on suspend/resume
+ */
+struct via_video_regs {
+ u32 interruptflag; /* 200 */
+ u32 ramtab; /* 204 */
+ u32 alphawin_hvstart; /* 208 */
+ u32 alphawin_size; /* 20c */
+ u32 alphawin_ctl; /* 210 */
+ u32 crt_startaddr; /* 214 */
+ u32 crt_startaddr_2; /* 218 */
+ u32 alphafb_stride ; /* 21c */
+ u32 color_key; /* 220 */
+ u32 alphafb_addr; /* 224 */
+ u32 chroma_low; /* 228 */
+ u32 chroma_up; /* 22c */
+ u32 video1_ctl; /* 230 */
+ u32 video1_fetch; /* 234 */
+ u32 video1y_addr1; /* 238 */
+ u32 video1_stride; /* 23c */
+ u32 video1_hvstart; /* 240 */
+ u32 video1_size; /* 244 */
+ u32 video1y_addr2; /* 248 */
+ u32 video1_zoom; /* 24c */
+ u32 video1_mictl; /* 250 */
+ u32 video1y_addr0; /* 254 */
+ u32 video1_fifo; /* 258 */
+ u32 video1y_addr3; /* 25c */
+ u32 hi_control; /* 260 */
+ u32 snd_color_key; /* 264 */
+ u32 v3alpha_prefifo; /* 268 */
+ u32 v1_source_w_h; /* 26c */
+ u32 hi_transparent_color; /* 270 */
+ u32 v_display_temp; /* 274 :No use */
+ u32 v3alpha_fifo; /* 278 */
+ u32 v3_source_width; /* 27c */
+ u32 dummy1; /* 280 */
+ u32 video1_CSC1; /* 284 */
+ u32 video1_CSC2; /* 288 */
+ u32 video1u_addr0; /* 28c */
+ u32 video1_opqctl; /* 290 */
+ u32 video3_opqctl; /* 294 */
+ u32 compose; /* 298 */
+ u32 dummy2; /* 29c */
+ u32 video3_ctl; /* 2a0 */
+ u32 video3_addr0; /* 2a4 */
+ u32 video3_addr1; /* 2a8 */
+ u32 video3_stride; /* 2ac */
+ u32 video3_hvstart; /* 2b0 */
+ u32 video3_size; /* 2b4 */
+ u32 v3alpha_fetch; /* 2b8 */
+ u32 video3_zoom; /* 2bc */
+ u32 video3_mictl; /* 2c0 */
+ u32 video3_CSC1; /* 2c4 */
+ u32 video3_CSC2; /* 2c8 */
+ u32 v3_display_temp; /* 2cc */
+ u32 cursor_mode;
+ u32 cursor_pos;
+ u32 cursor_org;
+ u32 cursor_bg;
+ u32 cursor_fg;
+ u32 video1u_addr1; /* 2e4 */
+ u32 video1u_addr2; /* 2e8 */
+ u32 video1u_addr3; /* 2ec */
+ u32 video1v_addr0; /* 2f0 */
+ u32 video1v_addr1; /* 2f4 */
+ u32 video1v_addr2; /* 2f8 */
+ u32 video1v_addr3; /* 2fc */
+};
+
+
struct viafb_shared {
struct proc_dir_entry *proc_entry; /*viafb proc entry */
@@ -61,10 +132,11 @@ struct viafb_shared {
u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
u32 fg_color, u32 bg_color, u8 fill_rop);
- /* For suspend/resume */
- u32 saved_regs[0x100];
+ /* For suspend/resume */
+ struct via_video_regs saved_video_regs;
};
+
struct viafb_par {
u8 depth;
u32 vram_addr;
--
1.7.0.1
From: Harald Welte <[email protected]>
This patch alters viafb to use the proper Linux in-kernel API to access
PCI configuration space, rather than poking at I/O ports by itself.
Signed-off-by: Harald Welte <[email protected]>
---
drivers/video/via/hw.c | 64 +++++++++++++++++++++++++++++------------------
drivers/video/via/hw.h | 4 +-
2 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index ea0f8ec..f2e4bda 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -2542,24 +2542,37 @@ static void disable_second_display_channel(void)
viafb_write_reg_mask(CR6A, VIACR, BIT6, BIT6);
}
+static u_int16_t via_function3[] = {
+ CLE266_FUNCTION3, KM400_FUNCTION3, CN400_FUNCTION3, CN700_FUNCTION3,
+ CX700_FUNCTION3, KM800_FUNCTION3, KM890_FUNCTION3, P4M890_FUNCTION3,
+ P4M900_FUNCTION3, VX800_FUNCTION3, VX855_FUNCTION3,
+};
+
+/* Get the BIOS-configured framebuffer size from PCI configuration space
+ * of function 3 in the respective chipset */
int viafb_get_fb_size_from_pci(void)
{
- unsigned long configid, deviceid, FBSize = 0;
- int VideoMemSize;
- int DeviceFound = false;
-
- for (configid = 0x80000000; configid < 0x80010800; configid += 0x100) {
- outl(configid, (unsigned long)0xCF8);
- deviceid = (inl((unsigned long)0xCFC) >> 16) & 0xffff;
-
- switch (deviceid) {
- case CLE266:
- case KM400:
- outl(configid + 0xE0, (unsigned long)0xCF8);
- FBSize = inl((unsigned long)0xCFC);
- DeviceFound = true; /* Found device id */
- break;
+ int i;
+ u_int8_t offset = 0;
+ u_int32_t FBSize;
+ u_int32_t VideoMemSize;
+
+ /* search for the "FUNCTION3" device in this chipset */
+ for (i = 0; i < ARRAY_SIZE(via_function3); i++) {
+ struct pci_dev *pdev;
+
+ pdev = pci_get_device(PCI_VENDOR_ID_VIA, via_function3[i],
+ NULL);
+ if (!pdev)
+ continue;
+
+ DEBUG_MSG(KERN_INFO "Device ID = %x\n", pdev->device);
+ switch (pdev->device) {
+ case CLE266_FUNCTION3:
+ case KM400_FUNCTION3:
+ offset = 0xE0;
+ break;
case CN400_FUNCTION3:
case CN700_FUNCTION3:
case CX700_FUNCTION3:
@@ -2569,21 +2582,22 @@ int viafb_get_fb_size_from_pci(void)
case P4M900_FUNCTION3:
case VX800_FUNCTION3:
case VX855_FUNCTION3:
- /*case CN750_FUNCTION3: */
- outl(configid + 0xA0, (unsigned long)0xCF8);
- FBSize = inl((unsigned long)0xCFC);
- DeviceFound = true; /* Found device id */
- break;
-
- default:
+ /*case CN750_FUNCTION3: */
+ offset = 0xA0;
break;
}
-
- if (DeviceFound)
+
+ if (!offset)
break;
+
+ pci_read_config_dword(pdev, offset, &FBSize);
+ pci_dev_put(pdev);
}
- DEBUG_MSG(KERN_INFO "Device ID = %lx\n", deviceid);
+ if (!offset) {
+ printk(KERN_ERR "cannot determine framebuffer size\n");
+ return -EIO;
+ }
FBSize = FBSize & 0x00007000;
DEBUG_MSG(KERN_INFO "FB Size = %x\n", FBSize);
diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
index b874d95..f67a56a 100644
--- a/drivers/video/via/hw.h
+++ b/drivers/video/via/hw.h
@@ -822,8 +822,8 @@ struct iga2_crtc_timing {
};
/* device ID */
-#define CLE266 0x3123
-#define KM400 0x3205
+#define CLE266_FUNCTION3 0x3123
+#define KM400_FUNCTION3 0x3205
#define CN400_FUNCTION2 0x2259
#define CN400_FUNCTION3 0x3259
/* support VT3314 chipset */
--
1.7.0.1
Commit c3e25673843153ea75fda79a47cf12f10a25ca37 (viafb: 2D engine rewrite)
changed the setting of the GEMODE register so that the reserved bits are no
longer preserved. Fix that; at the same time, move this code to its own
function and restore the use of symbolic constants.
Signed-off-by: Jonathan Corbet <[email protected]>
---
drivers/video/via/accel.c | 48 ++++++++++++++++++++++++++++++---------------
1 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
index d5077df..78c0a2b 100644
--- a/drivers/video/via/accel.c
+++ b/drivers/video/via/accel.c
@@ -165,12 +165,41 @@ static int hw_bitblt_1(void __iomem *engine, u8 op, u32 width, u32 height,
return 0;
}
+/*
+ * Figure out an appropriate bytes-per-pixel setting.
+ */
+static int viafb_set_bpp(void __iomem *engine, u8 bpp)
+{
+ u32 gemode;
+
+ /* Preserve the reserved bits */
+ gemode = readl(engine + VIA_REG_GEMODE) & 0xfffffcff;
+ switch (bpp) {
+ case 8:
+ gemode |= VIA_GEM_8bpp;
+ break;
+ case 16:
+ gemode |= VIA_GEM_16bpp;
+ break;
+ case 32:
+ gemode |= VIA_GEM_32bpp;
+ break;
+ default:
+ printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n", bpp);
+ return -EINVAL;
+ }
+ writel(gemode, engine + VIA_REG_GEMODE);
+ return 0;
+}
+
+
static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
u32 fg_color, u32 bg_color, u8 fill_rop)
{
u32 ge_cmd = 0, tmp, i;
+ int ret;
if (!op || op > 3) {
printk(KERN_WARNING "hw_bitblt_2: Invalid operation: %d\n", op);
@@ -204,22 +233,9 @@ static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
}
}
- switch (dst_bpp) {
- case 8:
- tmp = 0x00000000;
- break;
- case 16:
- tmp = 0x00000100;
- break;
- case 32:
- tmp = 0x00000300;
- break;
- default:
- printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n",
- dst_bpp);
- return -EINVAL;
- }
- writel(tmp, engine + 0x04);
+ ret = viafb_set_bpp(engine, dst_bpp);
+ if (ret)
+ return ret;
if (op == VIA_BITBLT_FILL)
tmp = 0;
--
1.7.0.1
From: Harald Welte <[email protected]>
The current code executed from module_init() in viafb does not have
proper error checking and [partial] resoure release paths in case
an error happens half way through driver initialization.
This patch adresses the most obvious of those issues, such as a
leftover i2c bus if module_init (and thus module load) fails.
[jc: fixed merge conflicts]
Signed-off-by: Harald Welte <[email protected]>
---
drivers/video/via/viafbdev.c | 52 ++++++++++++++++++++++++++++++-----------
1 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 3028e7d..91bfe6d 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1,5 +1,5 @@
/*
- * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
* Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
* This program is free software; you can redistribute it and/or
@@ -1847,7 +1847,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
u32 default_xres, default_yres;
- int vmode_index;
+ int rc, vmode_index;
u32 viafb_par_length;
DEBUG_MSG(KERN_INFO "VIAFB PCI Probe!!\n");
@@ -1862,7 +1862,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
&pdev->dev);
if (!viafbinfo) {
printk(KERN_ERR"Could not allocate memory for viafb_info.\n");
- return -ENODEV;
+ return -ENOMEM;
}
viaparinfo = (struct viafb_par *)viafbinfo->par;
@@ -1886,7 +1886,9 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
viafb_dual_fb = 0;
/* Set up I2C bus stuff */
- viafb_create_i2c_bus(viaparinfo);
+ rc = viafb_create_i2c_bus(viaparinfo);
+ if (rc)
+ goto out_fb_release;
viafb_init_chip_info(pdev, ent);
viaparinfo->fbmem = pci_resource_start(pdev, 0);
@@ -1897,7 +1899,8 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
viaparinfo->memsize);
if (!viafbinfo->screen_base) {
printk(KERN_INFO "ioremap failed\n");
- return -ENOMEM;
+ rc = -EIO;
+ goto out_delete_i2c;
}
viafbinfo->fix.mmio_start = pci_resource_start(pdev, 1);
@@ -1988,8 +1991,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
if (!viafbinfo1) {
printk(KERN_ERR
"allocate the second framebuffer struct error\n");
- framebuffer_release(viafbinfo);
- return -ENOMEM;
+ goto out_delete_i2c;
}
viaparinfo1 = viafbinfo1->par;
memcpy(viaparinfo1, viaparinfo, viafb_par_length);
@@ -2044,21 +2046,26 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
viaparinfo->depth = fb_get_color_depth(&viafbinfo->var,
&viafbinfo->fix);
default_var.activate = FB_ACTIVATE_NOW;
- fb_alloc_cmap(&viafbinfo->cmap, 256, 0);
+ rc = fb_alloc_cmap(&viafbinfo->cmap, 256, 0);
+ if (rc)
+ goto out_fb1_release;
if (viafb_dual_fb && (viafb_primary_dev == LCD_Device)
&& (viaparinfo->chip_info->gfx_chip_name == UNICHROME_CLE266)) {
- if (register_framebuffer(viafbinfo1) < 0)
- return -EINVAL;
+ rc = register_framebuffer(viafbinfo1);
+ if (rc)
+ goto out_dealloc_cmap;
}
- if (register_framebuffer(viafbinfo) < 0)
- return -EINVAL;
+ rc = register_framebuffer(viafbinfo);
+ if (rc)
+ goto out_fb1_unreg_lcd_cle266;
if (viafb_dual_fb && ((viafb_primary_dev != LCD_Device)
|| (viaparinfo->chip_info->gfx_chip_name !=
UNICHROME_CLE266))) {
- if (register_framebuffer(viafbinfo1) < 0)
- return -EINVAL;
+ rc = register_framebuffer(viafbinfo1);
+ if (rc)
+ goto out_fb_unreg;
}
DEBUG_MSG(KERN_INFO "fb%d: %s frame buffer device %dx%d-%dbpp\n",
viafbinfo->node, viafbinfo->fix.id, default_var.xres,
@@ -2067,6 +2074,23 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
viafb_init_proc(&viaparinfo->shared->proc_entry);
viafb_init_dac(IGA2);
return 0;
+
+out_fb_unreg:
+ unregister_framebuffer(viafbinfo);
+out_fb1_unreg_lcd_cle266:
+ if (viafb_dual_fb && (viafb_primary_dev == LCD_Device)
+ && (viaparinfo->chip_info->gfx_chip_name == UNICHROME_CLE266))
+ unregister_framebuffer(viafbinfo1);
+out_dealloc_cmap:
+ fb_dealloc_cmap(&viafbinfo->cmap);
+out_fb1_release:
+ if (viafbinfo1)
+ framebuffer_release(viafbinfo1);
+out_delete_i2c:
+ viafb_delete_i2c_buss(viaparinfo);
+out_fb_release:
+ framebuffer_release(viafbinfo);
+ return rc;
}
static void __devexit via_pci_remove(struct pci_dev *pdev)
--
1.7.0.1
Eliminate volatile pointers and direct dereferencing of iomem
pointers. In terms of register operations it should be the same as
before.
Signed-off-by: Jonathan Corbet <[email protected]>
---
drivers/video/via/viafbdev.c | 142 +++++++++++++++++++++++-------------------
drivers/video/via/viafbdev.h | 74 ----------------------
2 files changed, 77 insertions(+), 139 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 684a5c4..f834440 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1845,14 +1845,82 @@ out_default:
#ifdef CONFIG_PM
+/*
+ * The registers to save and restore, listed in the requisite order. This
+ * is an adaption of Deepak Saxena's suspend/restore code, which had been
+ * annotated thusly:
+ *
+ * Following set of register restores is black magic takend
+ * from the VIA X driver. Most of it is from the LeaveVT() and
+ * EnterVT() path and some is gleaned by looking at other bits
+ * of code to figure out what registers to touch.
+ */
+static const u32 via_regs_to_save[] = {
+ 0x208, /* Alpha win/hardware icon location start */
+ 0x20c, /* " location end */
+ 0x210, /* Alpha window control */
+ 0x21c, /* Alpha stream frame buffer stride */
+ 0x220, /* Primary display color key */
+ 0x224, /* Alpha window/hardware icon FB start */
+ 0x228, /* Chroma key lower bound */
+ 0x22c, /* Chroma key upper bound */
+ 0x230, /* Video stream 1 control */
+ 0x234, /* Video window 1 fetch count */
+ 0x238, /* VW1 fetch buffer Y start address 1 */
+ 0x23c, /* VW1 FB stride */
+ 0x240, /* VW1 starting location */
+ 0x244, /* VW1 ending location */
+ 0x248, /* VW1 FB Y starting address 2 */
+ 0x24c, /* VW1 display zoom control */
+ 0x250, /* VW1 minify & interpolation control */
+ 0x254, /* VW1 FB Y starting address 0 */
+ 0x258, /* VW1 FIFO depth and threshold control */
+ 0x25c, /* VW1 starting location offset */
+ 0x26c, /* VW1 display count on screen control */
+ 0x284, /* VW1 color space conv and enhancement control 1 */
+ 0x288, /* " 2 */
+ 0x264, /* Second display color key */
+ 0x268, /* V3 and alpha win FIFO pre-threshold control */
+ 0x278, /* V3 and alpha win FIFO depth and thresh control */
+ 0x2c4, /* VW3 color space conv and enhancement control 1 */
+ 0x2c8, /* " 2 */
+ 0x27c, /* VW3 display count on screen control */
+ 0x2a0, /* Video stream 3 control */
+ 0x2a4, /* VW3 FB starting address 0 */
+ 0x2a8, /* " 1 */
+ 0x2ac, /* VW3 FB stride */
+ 0x2b0, /* VW3 start */
+ 0x2b4, /* VW3 end */
+ 0x2b8, /* VW3 and alpha window fetch count */
+ 0x2bc, /* VW3 display zoom control */
+ 0x2c0, /* VW3 minify and interpolation control */
+ 0x2c4, /* VW3 color space conv and enhancement control 1 (again) */
+ 0x2c8, /* " 2 */
+ 0x298, /* Compose output mode select */
+ 0x2d0, /* Cursor mode control */
+ 0x2d4, /* Cursor position */
+ 0x2d8, /* Cursor origin */
+ 0x2dc, /* Cursor background */
+ 0x2e0 /* Cursor foreground */
+};
+#define VIA_N_SAVED_REGS ARRAY_SIZE(via_regs_to_save)
+
+/*
+ * Previous version had this in shared, but one static location is
+ * as good as another.
+ */
+static u32 via_pm_saved_regs[VIA_N_SAVED_REGS];
+
static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
{
+ int i;
+ void __iomem *iomem = viaparinfo->shared->engine_mmio;
+
if (state.event == PM_EVENT_SUSPEND) {
acquire_console_sem();
- memcpy_fromio(&viaparinfo->shared->saved_video_regs,
- viaparinfo->shared->engine_mmio + 0x100,
- sizeof(struct via_video_regs));
+ for (i = 0; i < VIA_N_SAVED_REGS; i++)
+ via_pm_saved_regs[i] = readl(iomem + via_regs_to_save[i]);
fb_set_suspend(viafbinfo, 1);
@@ -1869,9 +1937,8 @@ static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
static int viafb_resume(struct pci_dev *pdev)
{
- volatile struct via_video_regs *viaVidEng =
- (volatile struct via_video_regs *)(viaparinfo->shared->engine_mmio + 0x200);
- struct via_video_regs *localVidEng = &viaparinfo->shared->saved_video_regs;
+ int i;
+ void __iomem *iomem = viaparinfo->shared->engine_mmio;
acquire_console_sem();
pci_set_power_state(pdev, PCI_D0);
@@ -1881,65 +1948,10 @@ static int viafb_resume(struct pci_dev *pdev)
pci_set_master(pdev);
/*
- * Following set of register restores is black magic takend
- * from the VIA X driver. Most of it is from the LeaveVT() and
- * EnterVT() path and some is gleaned by looking at other bits
- * of code to figure out what registers to touch.
- */
- viaVidEng->alphawin_hvstart = localVidEng->alphawin_hvstart;
- viaVidEng->alphawin_size = localVidEng->alphawin_size;
- viaVidEng->alphawin_ctl = localVidEng->alphawin_ctl;
- viaVidEng->alphafb_stride = localVidEng->alphafb_stride;
- viaVidEng->color_key = localVidEng->color_key;
- viaVidEng->alphafb_addr = localVidEng->alphafb_addr;
- viaVidEng->chroma_low = localVidEng->chroma_low;
- viaVidEng->chroma_up = localVidEng->chroma_up;
-
- /*VT3314 only has V3*/
- viaVidEng->video1_ctl = localVidEng->video1_ctl;
- viaVidEng->video1_fetch = localVidEng->video1_fetch;
- viaVidEng->video1y_addr1 = localVidEng->video1y_addr1;
- viaVidEng->video1_stride = localVidEng->video1_stride;
- viaVidEng->video1_hvstart = localVidEng->video1_hvstart;
- viaVidEng->video1_size = localVidEng->video1_size;
- viaVidEng->video1y_addr2 = localVidEng->video1y_addr2;
- viaVidEng->video1_zoom = localVidEng->video1_zoom;
- viaVidEng->video1_mictl = localVidEng->video1_mictl;
- viaVidEng->video1y_addr0 = localVidEng->video1y_addr0;
- viaVidEng->video1_fifo = localVidEng->video1_fifo;
- viaVidEng->video1y_addr3 = localVidEng->video1y_addr3;
- viaVidEng->v1_source_w_h = localVidEng->v1_source_w_h ;
- viaVidEng->video1_CSC1 = localVidEng->video1_CSC1;
- viaVidEng->video1_CSC2 = localVidEng->video1_CSC2;
-
- viaVidEng->snd_color_key = localVidEng->snd_color_key;
- viaVidEng->v3alpha_prefifo = localVidEng->v3alpha_prefifo;
- viaVidEng->v3alpha_fifo = localVidEng->v3alpha_fifo;
- viaVidEng->video3_CSC2 = localVidEng->video3_CSC2;
- viaVidEng->video3_CSC2 = localVidEng->video3_CSC2;
- viaVidEng->v3_source_width = localVidEng->v3_source_width;
- viaVidEng->video3_ctl = localVidEng->video3_ctl;
- viaVidEng->video3_addr0 = localVidEng->video3_addr0;
- viaVidEng->video3_addr1 = localVidEng->video3_addr1;
- viaVidEng->video3_stride = localVidEng->video3_stride;
- viaVidEng->video3_hvstart = localVidEng->video3_hvstart;
- viaVidEng->video3_size = localVidEng->video3_size;
- viaVidEng->v3alpha_fetch = localVidEng->v3alpha_fetch;
- viaVidEng->video3_zoom = localVidEng->video3_zoom;
- viaVidEng->video3_mictl = localVidEng->video3_mictl;
- viaVidEng->video3_CSC1 = localVidEng->video3_CSC1;
- viaVidEng->video3_CSC2 = localVidEng->video3_CSC2;
- viaVidEng->compose = localVidEng->compose;
-
- /*
- * This _might_ not be needed, likely text mode cursor
+ * Restore registers.
*/
- viaVidEng->cursor_mode = localVidEng->cursor_mode;
- viaVidEng->cursor_pos = localVidEng->cursor_pos;
- viaVidEng->cursor_org = localVidEng->cursor_org;
- viaVidEng->cursor_bg = localVidEng->cursor_bg;
- viaVidEng->cursor_fg = localVidEng->cursor_fg;
-
+ for (i = 0; i < VIA_N_SAVED_REGS; i++)
+ writel(via_pm_saved_regs[i], iomem + via_regs_to_save[i]);
fb_set_suspend(viafbinfo, 0);
fail:
@@ -1947,7 +1959,7 @@ fail:
return 0;
}
-#endif
+#endif /* CONFIG_PM */
static int __devinit via_pci_probe(struct pci_dev *pdev,
diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
index 23b4afd..a108085 100644
--- a/drivers/video/via/viafbdev.h
+++ b/drivers/video/via/viafbdev.h
@@ -39,77 +39,6 @@
#define VIAFB_NUM_I2C 5
-/*
- * From VIA X Driver; these are the registers we save on suspend/resume
- */
-struct via_video_regs {
- u32 interruptflag; /* 200 */
- u32 ramtab; /* 204 */
- u32 alphawin_hvstart; /* 208 */
- u32 alphawin_size; /* 20c */
- u32 alphawin_ctl; /* 210 */
- u32 crt_startaddr; /* 214 */
- u32 crt_startaddr_2; /* 218 */
- u32 alphafb_stride ; /* 21c */
- u32 color_key; /* 220 */
- u32 alphafb_addr; /* 224 */
- u32 chroma_low; /* 228 */
- u32 chroma_up; /* 22c */
- u32 video1_ctl; /* 230 */
- u32 video1_fetch; /* 234 */
- u32 video1y_addr1; /* 238 */
- u32 video1_stride; /* 23c */
- u32 video1_hvstart; /* 240 */
- u32 video1_size; /* 244 */
- u32 video1y_addr2; /* 248 */
- u32 video1_zoom; /* 24c */
- u32 video1_mictl; /* 250 */
- u32 video1y_addr0; /* 254 */
- u32 video1_fifo; /* 258 */
- u32 video1y_addr3; /* 25c */
- u32 hi_control; /* 260 */
- u32 snd_color_key; /* 264 */
- u32 v3alpha_prefifo; /* 268 */
- u32 v1_source_w_h; /* 26c */
- u32 hi_transparent_color; /* 270 */
- u32 v_display_temp; /* 274 :No use */
- u32 v3alpha_fifo; /* 278 */
- u32 v3_source_width; /* 27c */
- u32 dummy1; /* 280 */
- u32 video1_CSC1; /* 284 */
- u32 video1_CSC2; /* 288 */
- u32 video1u_addr0; /* 28c */
- u32 video1_opqctl; /* 290 */
- u32 video3_opqctl; /* 294 */
- u32 compose; /* 298 */
- u32 dummy2; /* 29c */
- u32 video3_ctl; /* 2a0 */
- u32 video3_addr0; /* 2a4 */
- u32 video3_addr1; /* 2a8 */
- u32 video3_stride; /* 2ac */
- u32 video3_hvstart; /* 2b0 */
- u32 video3_size; /* 2b4 */
- u32 v3alpha_fetch; /* 2b8 */
- u32 video3_zoom; /* 2bc */
- u32 video3_mictl; /* 2c0 */
- u32 video3_CSC1; /* 2c4 */
- u32 video3_CSC2; /* 2c8 */
- u32 v3_display_temp; /* 2cc */
- u32 cursor_mode;
- u32 cursor_pos;
- u32 cursor_org;
- u32 cursor_bg;
- u32 cursor_fg;
- u32 video1u_addr1; /* 2e4 */
- u32 video1u_addr2; /* 2e8 */
- u32 video1u_addr3; /* 2ec */
- u32 video1v_addr0; /* 2f0 */
- u32 video1v_addr1; /* 2f4 */
- u32 video1v_addr2; /* 2f8 */
- u32 video1v_addr3; /* 2fc */
-};
-
-
struct viafb_shared {
struct proc_dir_entry *proc_entry; /*viafb proc entry */
@@ -131,9 +60,6 @@ struct viafb_shared {
u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
u32 fg_color, u32 bg_color, u8 fill_rop);
-
- /* For suspend/resume */
- struct via_video_regs saved_video_regs;
};
--
1.7.0.1
The code is only known to work there, and is strongly suspected to not work
on other chipsets.
Signed-off-by: Jonathan Corbet <[email protected]>
---
drivers/video/via/viafbdev.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index f834440..2e70c79 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1916,6 +1916,12 @@ static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
int i;
void __iomem *iomem = viaparinfo->shared->engine_mmio;
+/*
+ * This code is currently only known to work on VX855
+ */
+ if (viaparinfo->shared->chip_info.gfx_chip_name != UNICHROME_VX855)
+ return -ENOTSUPP;
+
if (state.event == PM_EVENT_SUSPEND) {
acquire_console_sem();
@@ -1940,6 +1946,12 @@ static int viafb_resume(struct pci_dev *pdev)
int i;
void __iomem *iomem = viaparinfo->shared->engine_mmio;
+/*
+ * This code is currently only known to work on VX855
+ */
+ if (viaparinfo->shared->chip_info.gfx_chip_name != UNICHROME_VX855)
+ return -ENOTSUPP;
+
acquire_console_sem();
pci_set_power_state(pdev, PCI_D0);
pci_restore_state(pdev);
--
1.7.0.1
From: Deepak Saxena <[email protected]>
Signed-off-by: Deepak Saxena <[email protected]>
---
drivers/video/via/viafbdev.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 56d5416..684a5c4 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1940,15 +1940,6 @@ static int viafb_resume(struct pci_dev *pdev)
viaVidEng->cursor_bg = localVidEng->cursor_bg;
viaVidEng->cursor_fg = localVidEng->cursor_fg;
- /*
- * Magic register dance to make cursor reappear.
- *
- * Currently hardcoded settings need to clean this all
- * up later...
- */
- viaVidEng->hi_control = 0xb6000005;
- viaVidEng->compose |= 0xc0000000;
-
fb_set_suspend(viafbinfo, 0);
fail:
--
1.7.0.1
From: Paul Fox <[email protected]>
Signed-off-by: Paul Fox <[email protected]>
---
drivers/video/via/viafbdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 88298e1..5a5964f 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1852,7 +1852,7 @@ static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
memcpy_fromio(viaparinfo->shared->saved_regs,
viaparinfo->shared->engine_mmio + 0x100,
- 0xff * sizeof(u32));
+ 0x100 * sizeof(u32));
fb_set_suspend(viafbinfo, 1);
--
1.7.0.1
From: Paul Fox <[email protected]>
---
drivers/video/via/via_i2c.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
index 18bbbdd..86e6e8d 100644
--- a/drivers/video/via/via_i2c.c
+++ b/drivers/video/via/via_i2c.c
@@ -26,7 +26,7 @@ static void via_i2c_setscl(void *data, int state)
u8 val;
struct via_i2c_adap_cfg *adap_data = data;
- printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
+ DEBUG_MSG(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
adap_data->ioport_index, adap_data->io_port);
val = viafb_read_reg(adap_data->io_port,
adap_data->ioport_index) & 0xF0;
@@ -140,7 +140,7 @@ static int create_i2c_bus(struct i2c_adapter *adapter,
struct via_i2c_adap_cfg *adap_cfg,
struct pci_dev *pdev)
{
- printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
+ DEBUG_MSG(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
algo->setsda = via_i2c_setsda;
algo->setscl = via_i2c_setscl;
@@ -182,7 +182,7 @@ static struct via_i2c_adap_cfg adap_configs[] = {
int viafb_create_i2c_busses(struct viafb_par *viapar)
{
int i, ret;
- printk(KERN_DEBUG "%s: entering\n", __FUNCTION__);
+ DEBUG_MSG(KERN_DEBUG "%s: entering\n", __FUNCTION__);
for (i = 0; i < VIAFB_NUM_I2C; i++) {
struct via_i2c_adap_cfg *adap_cfg = &adap_configs[i];
--
1.7.0.1
From: Deepak Saxena <[email protected]>
This patch adds minimal support for suspend/resume of the
VIA framebuffer device. It requires a version of OFW
that restores the video mode.
This patch is OLPC-specific as the proper upstream solution
is to move the VIA video path to using the kernel modesetting
infrastructure and doing a proper save/restore in the kernel.
[jc: extensive changes for 2.6.34 merge]
Signed-off-by: Deepak Saxena <[email protected]>
---
drivers/video/via/viafbdev.c | 51 ++++++++++++++++++++++++++++++++++++++++++
drivers/video/via/viafbdev.h | 3 ++
2 files changed, 54 insertions(+), 0 deletions(-)
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index ea3018e..88298e1 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1843,6 +1843,53 @@ out_default:
*yres = 480;
}
+
+#ifdef CONFIG_PM
+static int viafb_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+ if (state.event == PM_EVENT_SUSPEND) {
+ acquire_console_sem();
+
+ memcpy_fromio(viaparinfo->shared->saved_regs,
+ viaparinfo->shared->engine_mmio + 0x100,
+ 0xff * sizeof(u32));
+
+ fb_set_suspend(viafbinfo, 1);
+
+ viafb_sync(viafbinfo);
+
+ pci_save_state(pdev);
+ pci_disable_device(pdev);
+ pci_set_power_state(pdev, pci_choose_state(pdev, state));
+ release_console_sem();
+ }
+
+ return 0;
+}
+
+static int viafb_resume(struct pci_dev *pdev)
+{
+ acquire_console_sem();
+ pci_set_power_state(pdev, PCI_D0);
+ pci_restore_state(pdev);
+ if (pci_enable_device(pdev))
+ goto fail;
+ pci_set_master(pdev);
+
+ memcpy_toio(viaparinfo->shared->engine_mmio + 0x100,
+ viaparinfo->shared->saved_regs,
+ 0x100 * sizeof(u32));
+
+ fb_set_suspend(viafbinfo, 0);
+
+fail:
+ release_console_sem();
+ return 0;
+}
+
+#endif
+
+
static int __devinit via_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
@@ -2219,6 +2266,10 @@ static struct pci_driver viafb_driver = {
.id_table = viafb_pci_table,
.probe = via_pci_probe,
.remove = __devexit_p(via_pci_remove),
+#ifdef CONFIG_PM
+ .suspend = viafb_suspend,
+ .resume = viafb_resume,
+#endif
};
static int __init viafb_init(void)
diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
index 1dab97b..46d41af 100644
--- a/drivers/video/via/viafbdev.h
+++ b/drivers/video/via/viafbdev.h
@@ -60,6 +60,9 @@ struct viafb_shared {
u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
u32 fg_color, u32 bg_color, u8 fill_rop);
+
+ /* For suspend/resume */
+ u32 saved_regs[0x100];
};
struct viafb_par {
--
1.7.0.1
From: Harald Welte <[email protected]>
This patch changes the way how the various I2C busses are used internally
inside the viafb driver: Previosuly, only a single i2c_adapter was created,
even thougt two different hardware I2C busses are accessed: A structure member
in a global variable was modified to indicate the bus to be used.
Now, all existing hardware busses are registered with the i2c core, and the
viafb_i2c_{read,write}byte[s]() function take the adapter number as function
call parameter, rather than referring to the global structure member.
[jc: even more painful merge with mainline changes ->2.6.34]
[jc: painful merge with OLPC changes]
Signed-off-by: Harald Welte <[email protected]>
Signed-off-by: Jonathan Corbet <[email protected]>
---
drivers/video/via/dvi.c | 35 ++++-----
drivers/video/via/lcd.c | 15 ++--
drivers/video/via/via_i2c.c | 172 ++++++++++++++++++++++++++----------------
drivers/video/via/via_i2c.h | 43 +++++++----
drivers/video/via/viafbdev.c | 6 +-
drivers/video/via/viafbdev.h | 4 +-
drivers/video/via/vt1636.c | 36 ++++-----
drivers/video/via/vt1636.h | 2 +-
8 files changed, 181 insertions(+), 132 deletions(-)
diff --git a/drivers/video/via/dvi.c b/drivers/video/via/dvi.c
index 67b3693..00d001e 100644
--- a/drivers/video/via/dvi.c
+++ b/drivers/video/via/dvi.c
@@ -96,7 +96,7 @@ int viafb_tmds_trasmitter_identify(void)
viaparinfo->chip_info->tmds_chip_info.tmds_chip_name = VT1632_TMDS;
viaparinfo->chip_info->
tmds_chip_info.tmds_chip_slave_addr = VT1632_TMDS_I2C_ADDR;
- viaparinfo->chip_info->tmds_chip_info.i2c_port = I2CPORTINDEX;
+ viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_31;
if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID) != FAIL) {
/*
* Currently only support 12bits,dual edge,add 24bits mode later
@@ -110,7 +110,7 @@ int viafb_tmds_trasmitter_identify(void)
viaparinfo->chip_info->tmds_chip_info.i2c_port);
return OK;
} else {
- viaparinfo->chip_info->tmds_chip_info.i2c_port = GPIOPORTINDEX;
+ viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_2C;
if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID)
!= FAIL) {
tmds_register_write(0x08, 0x3b);
@@ -160,32 +160,26 @@ int viafb_tmds_trasmitter_identify(void)
static void tmds_register_write(int index, u8 data)
{
- viaparinfo->shared->i2c_stuff.i2c_port =
- viaparinfo->chip_info->tmds_chip_info.i2c_port;
-
- viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.
- tmds_chip_slave_addr, index,
- data);
+ viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
+ viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
+ index, data);
}
static int tmds_register_read(int index)
{
u8 data;
- viaparinfo->shared->i2c_stuff.i2c_port =
- viaparinfo->chip_info->tmds_chip_info.i2c_port;
- viafb_i2c_readbyte((u8) viaparinfo->chip_info->
- tmds_chip_info.tmds_chip_slave_addr,
- (u8) index, &data);
+ viafb_i2c_readbyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
+ (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
+ (u8) index, &data);
return data;
}
static int tmds_register_read_bytes(int index, u8 *buff, int buff_len)
{
- viaparinfo->shared->i2c_stuff.i2c_port =
- viaparinfo->chip_info->tmds_chip_info.i2c_port;
- viafb_i2c_readbytes((u8) viaparinfo->chip_info->tmds_chip_info.
- tmds_chip_slave_addr, (u8) index, buff, buff_len);
+ viafb_i2c_readbytes(viaparinfo->chip_info->tmds_chip_info.i2c_port,
+ (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
+ (u8) index, buff, buff_len);
return 0;
}
@@ -648,9 +642,10 @@ void viafb_dvi_enable(void)
else
data = 0x37;
viafb_i2c_writebyte(viaparinfo->chip_info->
- tmds_chip_info.
- tmds_chip_slave_addr,
- 0x08, data);
+ tmds_chip_info.i2c_port,
+ viaparinfo->chip_info->
+ tmds_chip_info.tmds_chip_slave_addr,
+ 0x08, data);
}
}
}
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 37a9746..fc27534 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -212,16 +212,14 @@ int viafb_lvds_trasmitter_identify(void)
if (machine_is_olpc())
return FAIL;
#endif
- viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
- if (viafb_lvds_identify_vt1636()) {
- viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
+ if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_31)) {
+ viaparinfo->chip_info->lvds_chip_info.i2c_port = VIA_I2C_ADAP_31;
DEBUG_MSG(KERN_INFO
"Found VIA VT1636 LVDS on port i2c 0x31 \n");
} else {
- viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
- if (viafb_lvds_identify_vt1636()) {
+ if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_26)) {
viaparinfo->chip_info->lvds_chip_info.i2c_port =
- GPIOPORTINDEX;
+ VIA_I2C_ADAP_2C;
DEBUG_MSG(KERN_INFO
"Found VIA VT1636 LVDS on port gpio 0x2c \n");
}
@@ -485,9 +483,8 @@ static int lvds_register_read(int index)
{
u8 data;
- viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
- viafb_i2c_readbyte((u8) viaparinfo->chip_info->
- lvds_chip_info.lvds_chip_slave_addr,
+ viafb_i2c_readbyte(VIA_I2C_ADAP_2C,
+ (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
(u8) index, &data);
return data;
}
diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
index 15543e9..18bbbdd 100644
--- a/drivers/video/via/via_i2c.c
+++ b/drivers/video/via/via_i2c.c
@@ -1,5 +1,5 @@
/*
- * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
* Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
* This program is free software; you can redistribute it and/or
@@ -24,40 +24,44 @@
static void via_i2c_setscl(void *data, int state)
{
u8 val;
- struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+ struct via_i2c_adap_cfg *adap_data = data;
- val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
+ printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
+ adap_data->ioport_index, adap_data->io_port);
+ val = viafb_read_reg(adap_data->io_port,
+ adap_data->ioport_index) & 0xF0;
if (state)
val |= 0x20;
else
val &= ~0x20;
- switch (via_i2c_chan->i2c_port) {
- case I2CPORTINDEX:
+ switch (adap_data->type) {
+ case VIA_I2C_I2C:
val |= 0x01;
break;
- case GPIOPORTINDEX:
+ case VIA_I2C_GPIO:
val |= 0x80;
break;
default:
- DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
+ DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
}
- viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
+ viafb_write_reg(adap_data->ioport_index,
+ adap_data->io_port, val);
}
static int via_i2c_getscl(void *data)
{
- struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+ struct via_i2c_adap_cfg *adap_data = data;
- if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x08)
+ if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x08)
return 1;
return 0;
}
static int via_i2c_getsda(void *data)
{
- struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+ struct via_i2c_adap_cfg *adap_data = data;
- if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x04)
+ if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x04)
return 1;
return 0;
}
@@ -65,27 +69,29 @@ static int via_i2c_getsda(void *data)
static void via_i2c_setsda(void *data, int state)
{
u8 val;
- struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
+ struct via_i2c_adap_cfg *adap_data = data;
- val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
+ val = viafb_read_reg(adap_data->io_port,
+ adap_data->ioport_index) & 0xF0;
if (state)
val |= 0x10;
else
val &= ~0x10;
- switch (via_i2c_chan->i2c_port) {
- case I2CPORTINDEX:
+ switch (adap_data->type) {
+ case VIA_I2C_I2C:
val |= 0x01;
break;
- case GPIOPORTINDEX:
+ case VIA_I2C_GPIO:
val |= 0x40;
break;
default:
- DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
+ DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
}
- viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
+ viafb_write_reg(adap_data->ioport_index,
+ adap_data->io_port, val);
}
-int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
+int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata)
{
u8 mm1[] = {0x00};
struct i2c_msg msgs[2];
@@ -97,12 +103,11 @@ int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
mm1[0] = index;
msgs[0].len = 1; msgs[1].len = 1;
msgs[0].buf = mm1; msgs[1].buf = pdata;
- i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
-
- return 0;
+ return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
+ msgs, 2);
}
-int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
+int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data)
{
u8 msg[2] = { index, data };
struct i2c_msg msgs;
@@ -111,10 +116,11 @@ int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
msgs.addr = slave_addr / 2;
msgs.len = 2;
msgs.buf = msg;
- return i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, &msgs, 1);
+ return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
+ &msgs, 1);
}
-int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
+int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len)
{
u8 mm1[] = {0x00};
struct i2c_msg msgs[2];
@@ -125,53 +131,89 @@ int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
mm1[0] = index;
msgs[0].len = 1; msgs[1].len = buff_len;
msgs[0].buf = mm1; msgs[1].buf = buff;
- i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
- return 0;
+ return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
+ msgs, 2);
}
-int viafb_create_i2c_bus(void *viapar)
+static int create_i2c_bus(struct i2c_adapter *adapter,
+ struct i2c_algo_bit_data *algo,
+ struct via_i2c_adap_cfg *adap_cfg,
+ struct pci_dev *pdev)
{
- int ret;
- struct via_i2c_stuff *i2c_stuff =
- &((struct viafb_par *)viapar)->shared->i2c_stuff;
-
- strcpy(i2c_stuff->adapter.name, "via_i2c");
- i2c_stuff->i2c_port = 0x0;
- i2c_stuff->adapter.owner = THIS_MODULE;
- i2c_stuff->adapter.id = 0x01FFFF;
- i2c_stuff->adapter.class = 0;
- i2c_stuff->adapter.algo_data = &i2c_stuff->algo;
- i2c_stuff->adapter.dev.parent = NULL;
- i2c_stuff->algo.setsda = via_i2c_setsda;
- i2c_stuff->algo.setscl = via_i2c_setscl;
- i2c_stuff->algo.getsda = via_i2c_getsda;
- i2c_stuff->algo.getscl = via_i2c_getscl;
- i2c_stuff->algo.udelay = 40;
- i2c_stuff->algo.timeout = 20;
- i2c_stuff->algo.data = i2c_stuff;
-
- i2c_set_adapdata(&i2c_stuff->adapter, i2c_stuff);
+ printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
+
+ algo->setsda = via_i2c_setsda;
+ algo->setscl = via_i2c_setscl;
+ algo->getsda = via_i2c_getsda;
+ algo->getscl = via_i2c_getscl;
+ algo->udelay = 40;
+ algo->timeout = 20;
+ algo->data = adap_cfg;
+
+ sprintf(adapter->name, "viafb i2c io_port idx 0x%02x",
+ adap_cfg->ioport_index);
+ adapter->owner = THIS_MODULE;
+ adapter->id = 0x01FFFF;
+ adapter->class = I2C_CLASS_DDC;
+ adapter->algo_data = algo;
+ if (pdev)
+ adapter->dev.parent = &pdev->dev;
+ else
+ adapter->dev.parent = NULL;
+ //i2c_set_adapdata(adapter, adap_cfg);
/* Raise SCL and SDA */
- i2c_stuff->i2c_port = I2CPORTINDEX;
- via_i2c_setsda(i2c_stuff, 1);
- via_i2c_setscl(i2c_stuff, 1);
-
- i2c_stuff->i2c_port = GPIOPORTINDEX;
- via_i2c_setsda(i2c_stuff, 1);
- via_i2c_setscl(i2c_stuff, 1);
+ via_i2c_setsda(adap_cfg, 1);
+ via_i2c_setscl(adap_cfg, 1);
udelay(20);
- ret = i2c_bit_add_bus(&i2c_stuff->adapter);
- if (ret == 0)
- DEBUG_MSG("I2C bus %s registered.\n", i2c_stuff->adapter.name);
- else
- DEBUG_MSG("Failed to register I2C bus %s.\n",
- i2c_stuff->adapter.name);
- return ret;
+ return i2c_bit_add_bus(adapter);
+}
+
+static struct via_i2c_adap_cfg adap_configs[] = {
+ [VIA_I2C_ADAP_26] = { VIA_I2C_I2C, VIASR, 0x26 },
+ [VIA_I2C_ADAP_31] = { VIA_I2C_I2C, VIASR, 0x31 },
+ [VIA_I2C_ADAP_25] = { VIA_I2C_GPIO, VIASR, 0x25 },
+ [VIA_I2C_ADAP_2C] = { VIA_I2C_GPIO, VIASR, 0x2c },
+ [VIA_I2C_ADAP_3D] = { VIA_I2C_GPIO, VIASR, 0x3d },
+ { 0, 0, 0 }
+};
+
+int viafb_create_i2c_busses(struct viafb_par *viapar)
+{
+ int i, ret;
+ printk(KERN_DEBUG "%s: entering\n", __FUNCTION__);
+
+ for (i = 0; i < VIAFB_NUM_I2C; i++) {
+ struct via_i2c_adap_cfg *adap_cfg = &adap_configs[i];
+ struct via_i2c_stuff *i2c_stuff = &viapar->shared->i2c_stuff[i];
+
+ if (adap_cfg->type == 0)
+ break;
+
+ ret = create_i2c_bus(&i2c_stuff->adapter,
+ &i2c_stuff->algo, adap_cfg,
+ NULL); //FIXME: PCIDEV
+ if (ret < 0) {
+ printk(KERN_ERR "viafb: cannot create i2c bus %u:%d\n",
+ i, ret);
+ //FIXME: properly release previous busses
+ return ret;
+ }
+ }
+
+ return 0;
}
-void viafb_delete_i2c_buss(void *par)
+void viafb_delete_i2c_busses(struct viafb_par *par)
{
- i2c_del_adapter(&((struct viafb_par *)par)->shared->i2c_stuff.adapter);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(par->shared->i2c_stuff); i++) {
+ struct via_i2c_stuff *i2c_stuff = &par->shared->i2c_stuff[i];
+ /* only remove those entries in the array that we've
+ * actually used (and thus initialized algo_data) */
+ if (i2c_stuff->adapter.algo_data == &i2c_stuff->algo)
+ i2c_del_adapter(&i2c_stuff->adapter);
+ }
}
diff --git a/drivers/video/via/via_i2c.h b/drivers/video/via/via_i2c.h
index 3a13242..00ed978 100644
--- a/drivers/video/via/via_i2c.h
+++ b/drivers/video/via/via_i2c.h
@@ -1,5 +1,5 @@
/*
- * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
* Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
* This program is free software; you can redistribute it and/or
@@ -24,23 +24,38 @@
#include <linux/i2c.h>
#include <linux/i2c-algo-bit.h>
+enum via_i2c_type {
+ VIA_I2C_NONE,
+ VIA_I2C_I2C,
+ VIA_I2C_GPIO,
+};
+
+/* private data for each adapter */
+struct via_i2c_adap_cfg {
+ enum via_i2c_type type;
+ u_int16_t io_port;
+ u_int8_t ioport_index;
+};
+
struct via_i2c_stuff {
u16 i2c_port; /* GPIO or I2C port */
struct i2c_adapter adapter;
struct i2c_algo_bit_data algo;
};
-#define I2CPORT 0x3c4
-#define I2CPORTINDEX 0x31
-#define GPIOPORT 0x3C4
-#define GPIOPORTINDEX 0x2C
-#define I2C_BUS 1
-#define GPIO_BUS 2
-#define DELAYPORT 0x3C3
-
-int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata);
-int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data);
-int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len);
-int viafb_create_i2c_bus(void *par);
-void viafb_delete_i2c_buss(void *par);
+enum viafb_i2c_adap {
+ VIA_I2C_ADAP_26,
+ VIA_I2C_ADAP_31,
+ VIA_I2C_ADAP_25,
+ VIA_I2C_ADAP_2C,
+ VIA_I2C_ADAP_3D,
+};
+
+int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata);
+int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data);
+int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len);
+
+struct viafb_par;
+int viafb_create_i2c_busses(struct viafb_par *par);
+void viafb_delete_i2c_busses(struct viafb_par *par);
#endif /* __VIA_I2C_H__ */
diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
index 4a34d4f..ea3018e 100644
--- a/drivers/video/via/viafbdev.c
+++ b/drivers/video/via/viafbdev.c
@@ -1886,7 +1886,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
viafb_dual_fb = 0;
/* Set up I2C bus stuff */
- rc = viafb_create_i2c_bus(viaparinfo);
+ rc = viafb_create_i2c_busses(viaparinfo);
if (rc)
goto out_fb_release;
@@ -2089,7 +2089,7 @@ out_fb1_release:
out_unmap_screen:
iounmap(viafbinfo->screen_base);
out_delete_i2c:
- viafb_delete_i2c_buss(viaparinfo);
+ viafb_delete_i2c_busses(viaparinfo);
out_fb_release:
framebuffer_release(viafbinfo);
return rc;
@@ -2105,7 +2105,7 @@ static void __devexit via_pci_remove(struct pci_dev *pdev)
iounmap((void *)viafbinfo->screen_base);
iounmap(viaparinfo->shared->engine_mmio);
- viafb_delete_i2c_buss(viaparinfo);
+ viafb_delete_i2c_busses(viaparinfo);
framebuffer_release(viafbinfo);
if (viafb_dual_fb)
diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
index 0c94d24..1dab97b 100644
--- a/drivers/video/via/viafbdev.h
+++ b/drivers/video/via/viafbdev.h
@@ -37,11 +37,13 @@
#define VERSION_OS 0 /* 0: for 32 bits OS, 1: for 64 bits OS */
#define VERSION_MINOR 4
+#define VIAFB_NUM_I2C 5
+
struct viafb_shared {
struct proc_dir_entry *proc_entry; /*viafb proc entry */
/* I2C stuff */
- struct via_i2c_stuff i2c_stuff;
+ struct via_i2c_stuff i2c_stuff[VIAFB_NUM_I2C];
/* All the information will be needed to set engine */
struct tmds_setting_information tmds_setting_info;
diff --git a/drivers/video/via/vt1636.c b/drivers/video/via/vt1636.c
index a6b3749..d75b5f0 100644
--- a/drivers/video/via/vt1636.c
+++ b/drivers/video/via/vt1636.c
@@ -27,9 +27,8 @@ u8 viafb_gpio_i2c_read_lvds(struct lvds_setting_information
{
u8 data;
- viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
- viafb_i2c_readbyte(plvds_chip_info->lvds_chip_slave_addr, index, &data);
-
+ viafb_i2c_readbyte(plvds_chip_info->i2c_port,
+ plvds_chip_info->lvds_chip_slave_addr, index, &data);
return data;
}
@@ -39,14 +38,13 @@ void viafb_gpio_i2c_write_mask_lvds(struct lvds_setting_information
{
int index, data;
- viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
-
index = io_data.Index;
data = viafb_gpio_i2c_read_lvds(plvds_setting_info, plvds_chip_info,
index);
data = (data & (~io_data.Mask)) | io_data.Data;
- viafb_i2c_writebyte(plvds_chip_info->lvds_chip_slave_addr, index, data);
+ viafb_i2c_writebyte(plvds_chip_info->i2c_port,
+ plvds_chip_info->lvds_chip_slave_addr, index, data);
}
void viafb_init_lvds_vt1636(struct lvds_setting_information
@@ -159,7 +157,7 @@ void viafb_disable_lvds_vt1636(struct lvds_setting_information
}
}
-bool viafb_lvds_identify_vt1636(void)
+bool viafb_lvds_identify_vt1636(u8 i2c_adapter)
{
u8 Buffer[2];
@@ -170,23 +168,23 @@ bool viafb_lvds_identify_vt1636(void)
VT1636_LVDS_I2C_ADDR;
/* Check vendor ID first: */
- viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
- lvds_chip_slave_addr,
- 0x00, &Buffer[0]);
- viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
- lvds_chip_slave_addr,
- 0x01, &Buffer[1]);
+ viafb_i2c_readbyte(i2c_adapter,
+ (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+ 0x00, &Buffer[0]);
+ viafb_i2c_readbyte(i2c_adapter,
+ (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+ 0x01, &Buffer[1]);
if (!((Buffer[0] == 0x06) && (Buffer[1] == 0x11)))
return false;
/* Check Chip ID: */
- viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
- lvds_chip_slave_addr,
- 0x02, &Buffer[0]);
- viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
- lvds_chip_slave_addr,
- 0x03, &Buffer[1]);
+ viafb_i2c_readbyte(i2c_adapter,
+ (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+ 0x02, &Buffer[0]);
+ viafb_i2c_readbyte(i2c_adapter,
+ (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
+ 0x03, &Buffer[1]);
if ((Buffer[0] == 0x45) && (Buffer[1] == 0x33)) {
viaparinfo->chip_info->lvds_chip_info.lvds_chip_name =
VT1636_LVDS;
diff --git a/drivers/video/via/vt1636.h b/drivers/video/via/vt1636.h
index 2a150c5..4c1314e 100644
--- a/drivers/video/via/vt1636.h
+++ b/drivers/video/via/vt1636.h
@@ -22,7 +22,7 @@
#ifndef _VT1636_H_
#define _VT1636_H_
#include "chip.h"
-bool viafb_lvds_identify_vt1636(void);
+bool viafb_lvds_identify_vt1636(u8 i2c_adapter);
void viafb_init_lvds_vt1636(struct lvds_setting_information
*plvds_setting_info, struct lvds_chip_information *plvds_chip_info);
void viafb_enable_lvds_vt1636(struct lvds_setting_information
--
1.7.0.1
From: Chris Ball <[email protected]>
The i2c transactions involved in detecting LVDS (9 seconds) and TMDS
(16 seconds) add an extra 25 seconds to viafb load time on the XO-1.5.
[jc: minor merge conflict fixed]
Signed-off-by: Chris Ball <[email protected]>
---
drivers/video/via/hw.c | 4 ++++
drivers/video/via/lcd.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 7be462e..47ba09a 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -2054,6 +2054,10 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
static void init_tmds_chip_info(void)
{
+#ifdef CONFIG_OLPC_XO_1_5
+ if (machine_is_olpc())
+ return;
+#endif
viafb_tmds_trasmitter_identify();
if (INTERFACE_NONE == viaparinfo->chip_info->tmds_chip_info.
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index e0e2310..37a9746 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -208,6 +208,10 @@ static bool lvds_identify_integratedlvds(void)
int viafb_lvds_trasmitter_identify(void)
{
+#ifdef CONFIG_OLPC_XO_1_5
+ if (machine_is_olpc())
+ return FAIL;
+#endif
viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
if (viafb_lvds_identify_vt1636()) {
viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
--
1.7.0.1
From: Chris Ball <[email protected]>
[jc: extensive merge conflict fixes]
Signed-off-by: Chris Ball <[email protected]>
---
drivers/video/via/hw.c | 1 +
drivers/video/via/ioctl.h | 2 +-
drivers/video/via/lcd.c | 10 ++++++++++
drivers/video/via/lcd.h | 2 ++
drivers/video/via/share.h | 8 ++++++++
drivers/video/via/viamode.c | 14 ++++++++++++++
6 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
index 963704e..7be462e 100644
--- a/drivers/video/via/hw.c
+++ b/drivers/video/via/hw.c
@@ -63,6 +63,7 @@ static struct pll_map pll_value[] = {
CX700_52_977M, VX855_52_977M},
{CLK_56_250M, CLE266_PLL_56_250M, K800_PLL_56_250M,
CX700_56_250M, VX855_56_250M},
+ {CLK_57_275M, 0, 0, 0, VX855_57_275M},
{CLK_60_466M, CLE266_PLL_60_466M, K800_PLL_60_466M,
CX700_60_466M, VX855_60_466M},
{CLK_61_500M, CLE266_PLL_61_500M, K800_PLL_61_500M,
diff --git a/drivers/video/via/ioctl.h b/drivers/video/via/ioctl.h
index de89980..c430fa2 100644
--- a/drivers/video/via/ioctl.h
+++ b/drivers/video/via/ioctl.h
@@ -75,7 +75,7 @@
/*SAMM operation flag*/
#define OP_SAMM 0x80
-#define LCD_PANEL_ID_MAXIMUM 22
+#define LCD_PANEL_ID_MAXIMUM 23
#define STATE_ON 0x1
#define STATE_OFF 0x0
diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 71e3200..e0e2310 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -456,6 +456,16 @@ static int fp_id_to_vindex(int panel_id)
viaparinfo->lvds_setting_info->LCDDithering = 1;
return VIA_RES_480X640;
break;
+ case 0x17:
+ /* OLPC XO-1.5 panel */
+ viaparinfo->lvds_setting_info->lcd_panel_hres = 1200;
+ viaparinfo->lvds_setting_info->lcd_panel_vres = 900;
+ viaparinfo->lvds_setting_info->lcd_panel_id =
+ LCD_PANEL_IDD_1200X900;
+ viaparinfo->lvds_setting_info->device_lcd_dualedge = 0;
+ viaparinfo->lvds_setting_info->LCDDithering = 0;
+ return VIA_RES_1200X900;
+ break;
default:
viaparinfo->lvds_setting_info->lcd_panel_hres = 800;
viaparinfo->lvds_setting_info->lcd_panel_vres = 600;
diff --git a/drivers/video/via/lcd.h b/drivers/video/via/lcd.h
index 071f47c..9762ec6 100644
--- a/drivers/video/via/lcd.h
+++ b/drivers/video/via/lcd.h
@@ -60,6 +60,8 @@
#define LCD_PANEL_IDB_1360X768 0x0B
/* Resolution: 480x640, Channel: single, Dithering: Enable */
#define LCD_PANEL_IDC_480X640 0x0C
+/* Resolution: 1200x900, Channel: single, Dithering: Disable */
+#define LCD_PANEL_IDD_1200X900 0x0D
extern int viafb_LCD2_ON;
diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
index 7cd03e2..ecbbf93 100644
--- a/drivers/video/via/share.h
+++ b/drivers/video/via/share.h
@@ -86,6 +86,7 @@
#define VIA_RES_1920X1200 38
#define VIA_RES_2048X1536 39
#define VIA_RES_480X640 40
+#define VIA_RES_1200X900 41
/*Reduce Blanking*/
#define VIA_RES_1360X768_RB 131
@@ -626,6 +627,10 @@
#define M1200X720_R60_HSP NEGATIVE
#define M1200X720_R60_VSP POSITIVE
+/* 1200x900@60 Sync Polarity (DCON) */
+#define M1200X900_R60_HSP NEGATIVE
+#define M1200X900_R60_VSP NEGATIVE
+
/* 1280x600@60 Sync Polarity (GTF Mode) */
#define M1280x600_R60_HSP NEGATIVE
#define M1280x600_R60_VSP POSITIVE
@@ -707,6 +712,7 @@
#define CLK_52_406M 52406000
#define CLK_52_977M 52977000
#define CLK_56_250M 56250000
+#define CLK_57_275M 57275000
#define CLK_60_466M 60466000
#define CLK_61_500M 61500000
#define CLK_65_000M 65000000
@@ -995,6 +1001,7 @@
#define VX855_52_406M 0x00580C03
#define VX855_52_977M 0x00940C05
#define VX855_56_250M 0x009D0C05
+#define VX855_57_275M 0x009D8C85 /* Used by XO panel */
#define VX855_60_466M 0x00A90C05
#define VX855_61_500M 0x00AC0C05
#define VX855_65_000M 0x006D0C03
@@ -1121,6 +1128,7 @@
#define RES_1600X1200_60HZ_PIXCLOCK 6172
#define RES_1600X1200_75HZ_PIXCLOCK 4938
#define RES_1280X720_60HZ_PIXCLOCK 13426
+#define RES_1200X900_60HZ_PIXCLOCK 17459
#define RES_1920X1080_60HZ_PIXCLOCK 5787
#define RES_1400X1050_60HZ_PIXCLOCK 8214
#define RES_1400X1050_75HZ_PIXCLOCK 6410
diff --git a/drivers/video/via/viamode.c b/drivers/video/via/viamode.c
index b74f8a6..46833b4 100644
--- a/drivers/video/via/viamode.c
+++ b/drivers/video/via/viamode.c
@@ -66,6 +66,7 @@ struct res_map_refresh res_map_refresh_tbl[] = {
{1088, 612, RES_1088X612_60HZ_PIXCLOCK, 60},
{1152, 720, RES_1152X720_60HZ_PIXCLOCK, 60},
{1200, 720, RES_1200X720_60HZ_PIXCLOCK, 60},
+ {1200, 900, RES_1200X900_60HZ_PIXCLOCK, 60},
{1280, 600, RES_1280X600_60HZ_PIXCLOCK, 60},
{1280, 720, RES_1280X720_50HZ_PIXCLOCK, 50},
{1280, 768, RES_1280X768_50HZ_PIXCLOCK, 50},
@@ -759,6 +760,16 @@ struct crt_mode_table CRTM1200x720[] = {
{1568, 1200, 1200, 368, 1256, 128, 746, 720, 720, 26, 721, 3} }
};
+/* 1200x900 (DCON) */
+struct crt_mode_table DCON1200x900[] = {
+ /* r_rate, vclk, hsp, vsp */
+ {REFRESH_60, CLK_57_275M, M1200X900_R60_HSP, M1200X900_R60_VSP,
+ /* The correct htotal is 1240, but this doesn't raster on VX855. */
+ /* Via suggested changing to a multiple of 16, hence 1264. */
+ /* HT, HA, HBS, HBE, HSS, HSE, VT, VA, VBS, VBE, VSS, VSE */
+ {1264, 1200, 1200, 64, 1211, 32, 912, 900, 900, 12, 901, 10} }
+};
+
/* 1280x600 (GTF) */
struct crt_mode_table CRTM1280x600[] = {
/* r_rate, vclk, hsp, vsp */
@@ -946,6 +957,9 @@ struct VideoModeTable CLE266Modes[] = {
/* Display : 1200x720 (GTF) */
{VIA_RES_1200X720, CRTM1200x720, ARRAY_SIZE(CRTM1200x720)},
+ /* Display : 1200x900 (DCON) */
+ {VIA_RES_1200X900, DCON1200x900, ARRAY_SIZE(DCON1200x900)},
+
/* Display : 1280x600 (GTF) */
{VIA_RES_1280X600, CRTM1280x600, ARRAY_SIZE(CRTM1280x600)},
--
1.7.0.1
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 <[email protected]>
---
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);
+
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:
+ 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 */
+
/* 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
--
1.7.0.1
Jonathan Corbet schrieb:
> From: Harald Welte <[email protected]>
>
> The current code executed from module_init() in viafb does not have
> proper error checking and [partial] resoure release paths in case
> an error happens half way through driver initialization.
>
> This patch adresses the most obvious of those issues, such as a
> leftover i2c bus if module_init (and thus module load) fails.
>
> [jc: fixed merge conflicts]
> Signed-off-by: Harald Welte <[email protected]>
> ---
> drivers/video/via/viafbdev.c | 52 ++++++++++++++++++++++++++++++-----------
> 1 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
> index 3028e7d..91bfe6d 100644
> --- a/drivers/video/via/viafbdev.c
> +++ b/drivers/video/via/viafbdev.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
> + * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
> * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
>
> * This program is free software; you can redistribute it and/or
> @@ -1847,7 +1847,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> {
> u32 default_xres, default_yres;
> - int vmode_index;
> + int rc, vmode_index;
> u32 viafb_par_length;
>
> DEBUG_MSG(KERN_INFO "VIAFB PCI Probe!!\n");
> @@ -1862,7 +1862,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> &pdev->dev);
> if (!viafbinfo) {
> printk(KERN_ERR"Could not allocate memory for viafb_info.\n");
> - return -ENODEV;
> + return -ENOMEM;
> }
>
> viaparinfo = (struct viafb_par *)viafbinfo->par;
> @@ -1886,7 +1886,9 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> viafb_dual_fb = 0;
>
> /* Set up I2C bus stuff */
> - viafb_create_i2c_bus(viaparinfo);
> + rc = viafb_create_i2c_bus(viaparinfo);
> + if (rc)
> + goto out_fb_release;
>
> viafb_init_chip_info(pdev, ent);
> viaparinfo->fbmem = pci_resource_start(pdev, 0);
> @@ -1897,7 +1899,8 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> viaparinfo->memsize);
> if (!viafbinfo->screen_base) {
> printk(KERN_INFO "ioremap failed\n");
> - return -ENOMEM;
> + rc = -EIO;
I don't know whether this is right (changing the return code) as Andrew
recommend a while ago:
"It should return -ENOMEM rather than -1, but that's minor."
So I did and now I wonder which one is correct?
> + goto out_delete_i2c;
> }
>
> viafbinfo->fix.mmio_start = pci_resource_start(pdev, 1);
> @@ -1988,8 +1991,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> if (!viafbinfo1) {
> printk(KERN_ERR
> "allocate the second framebuffer struct error\n");
> - framebuffer_release(viafbinfo);
> - return -ENOMEM;
rc = -ENOMEM;
is missing?
> + goto out_delete_i2c;
> }
> viaparinfo1 = viafbinfo1->par;
> memcpy(viaparinfo1, viaparinfo, viafb_par_length);
> @@ -2044,21 +2046,26 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> viaparinfo->depth = fb_get_color_depth(&viafbinfo->var,
> &viafbinfo->fix);
> default_var.activate = FB_ACTIVATE_NOW;
> - fb_alloc_cmap(&viafbinfo->cmap, 256, 0);
> + rc = fb_alloc_cmap(&viafbinfo->cmap, 256, 0);
> + if (rc)
> + goto out_fb1_release;
>
> if (viafb_dual_fb && (viafb_primary_dev == LCD_Device)
> && (viaparinfo->chip_info->gfx_chip_name == UNICHROME_CLE266)) {
> - if (register_framebuffer(viafbinfo1) < 0)
> - return -EINVAL;
> + rc = register_framebuffer(viafbinfo1);
> + if (rc)
> + goto out_dealloc_cmap;
> }
> - if (register_framebuffer(viafbinfo) < 0)
> - return -EINVAL;
> + rc = register_framebuffer(viafbinfo);
> + if (rc)
> + goto out_fb1_unreg_lcd_cle266;
>
> if (viafb_dual_fb && ((viafb_primary_dev != LCD_Device)
> || (viaparinfo->chip_info->gfx_chip_name !=
> UNICHROME_CLE266))) {
> - if (register_framebuffer(viafbinfo1) < 0)
> - return -EINVAL;
> + rc = register_framebuffer(viafbinfo1);
> + if (rc)
> + goto out_fb_unreg;
> }
> DEBUG_MSG(KERN_INFO "fb%d: %s frame buffer device %dx%d-%dbpp\n",
> viafbinfo->node, viafbinfo->fix.id, default_var.xres,
> @@ -2067,6 +2074,23 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> viafb_init_proc(&viaparinfo->shared->proc_entry);
> viafb_init_dac(IGA2);
> return 0;
> +
> +out_fb_unreg:
> + unregister_framebuffer(viafbinfo);
> +out_fb1_unreg_lcd_cle266:
> + if (viafb_dual_fb && (viafb_primary_dev == LCD_Device)
> + && (viaparinfo->chip_info->gfx_chip_name == UNICHROME_CLE266))
> + unregister_framebuffer(viafbinfo1);
> +out_dealloc_cmap:
> + fb_dealloc_cmap(&viafbinfo->cmap);
> +out_fb1_release:
> + if (viafbinfo1)
> + framebuffer_release(viafbinfo1);
> +out_delete_i2c:
> + viafb_delete_i2c_buss(viaparinfo);
> +out_fb_release:
> + framebuffer_release(viafbinfo);
> + return rc;
> }
>
> static void __devexit via_pci_remove(struct pci_dev *pdev)
Otherwise it looks okay.
Thanks,
Florian Tobias Schandinat
Hi,
something I am wondering about is whether we can't simply do:
viaparinfo->memsize = pci_resource_len(pdev, 0);
I suppose that this is not possible meaning that the pci len can be
longer than the actual memory but I just wanted to use the moment to
make sure.
Jonathan Corbet schrieb:
> From: Harald Welte <[email protected]>
>
> This patch alters viafb to use the proper Linux in-kernel API to access
> PCI configuration space, rather than poking at I/O ports by itself.
>
> Signed-off-by: Harald Welte <[email protected]>
> ---
> drivers/video/via/hw.c | 64 +++++++++++++++++++++++++++++------------------
> drivers/video/via/hw.h | 4 +-
> 2 files changed, 41 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
> index ea0f8ec..f2e4bda 100644
> --- a/drivers/video/via/hw.c
> +++ b/drivers/video/via/hw.c
> @@ -2542,24 +2542,37 @@ static void disable_second_display_channel(void)
> viafb_write_reg_mask(CR6A, VIACR, BIT6, BIT6);
> }
>
> +static u_int16_t via_function3[] = {
> + CLE266_FUNCTION3, KM400_FUNCTION3, CN400_FUNCTION3, CN700_FUNCTION3,
> + CX700_FUNCTION3, KM800_FUNCTION3, KM890_FUNCTION3, P4M890_FUNCTION3,
> + P4M900_FUNCTION3, VX800_FUNCTION3, VX855_FUNCTION3,
> +};
> +
> +/* Get the BIOS-configured framebuffer size from PCI configuration space
> + * of function 3 in the respective chipset */
> int viafb_get_fb_size_from_pci(void)
> {
> - unsigned long configid, deviceid, FBSize = 0;
> - int VideoMemSize;
> - int DeviceFound = false;
> -
> - for (configid = 0x80000000; configid < 0x80010800; configid += 0x100) {
> - outl(configid, (unsigned long)0xCF8);
> - deviceid = (inl((unsigned long)0xCFC) >> 16) & 0xffff;
> -
> - switch (deviceid) {
> - case CLE266:
> - case KM400:
> - outl(configid + 0xE0, (unsigned long)0xCF8);
> - FBSize = inl((unsigned long)0xCFC);
> - DeviceFound = true; /* Found device id */
> - break;
> + int i;
> + u_int8_t offset = 0;
> + u_int32_t FBSize;
> + u_int32_t VideoMemSize;
> +
> + /* search for the "FUNCTION3" device in this chipset */
> + for (i = 0; i < ARRAY_SIZE(via_function3); i++) {
Would it be possible to replace this loop with a lookup table that uses
the fact that we already know which IGP we have?
Yeah, that's not a bug it only feels a bit awkward to do it this way.
> + struct pci_dev *pdev;
> +
> + pdev = pci_get_device(PCI_VENDOR_ID_VIA, via_function3[i],
> + NULL);
> + if (!pdev)
> + continue;
> +
> + DEBUG_MSG(KERN_INFO "Device ID = %x\n", pdev->device);
>
> + switch (pdev->device) {
> + case CLE266_FUNCTION3:
> + case KM400_FUNCTION3:
> + offset = 0xE0;
> + break;
> case CN400_FUNCTION3:
> case CN700_FUNCTION3:
> case CX700_FUNCTION3:
> @@ -2569,21 +2582,22 @@ int viafb_get_fb_size_from_pci(void)
> case P4M900_FUNCTION3:
> case VX800_FUNCTION3:
> case VX855_FUNCTION3:
> - /*case CN750_FUNCTION3: */
> - outl(configid + 0xA0, (unsigned long)0xCF8);
> - FBSize = inl((unsigned long)0xCFC);
> - DeviceFound = true; /* Found device id */
> - break;
> -
> - default:
> + /*case CN750_FUNCTION3: */
> + offset = 0xA0;
> break;
> }
> -
> - if (DeviceFound)
> +
> + if (!offset)
> break;
> +
> + pci_read_config_dword(pdev, offset, &FBSize);
> + pci_dev_put(pdev);
> }
>
> - DEBUG_MSG(KERN_INFO "Device ID = %lx\n", deviceid);
> + if (!offset) {
> + printk(KERN_ERR "cannot determine framebuffer size\n");
> + return -EIO;
This function was not designed to return an error (memsize is not
checked). Either return a default value (let's say 8MB) or add a check
for memsize.
> + }
>
> FBSize = FBSize & 0x00007000;
> DEBUG_MSG(KERN_INFO "FB Size = %x\n", FBSize);
> diff --git a/drivers/video/via/hw.h b/drivers/video/via/hw.h
> index b874d95..f67a56a 100644
> --- a/drivers/video/via/hw.h
> +++ b/drivers/video/via/hw.h
> @@ -822,8 +822,8 @@ struct iga2_crtc_timing {
> };
>
> /* device ID */
> -#define CLE266 0x3123
> -#define KM400 0x3205
> +#define CLE266_FUNCTION3 0x3123
> +#define KM400_FUNCTION3 0x3205
> #define CN400_FUNCTION2 0x2259
> #define CN400_FUNCTION3 0x3259
> /* support VT3314 chipset */
Thanks,
Florian Tobias Schandinat
Jonathan Corbet schrieb:
> This was part of Harald's "make viafb a first-class citizen using
> pci_driver" patch, but somehow got dropped when that patch went into
> mainline.
Well it was dropped because the patch introducing the goto_error_out
logic wasn't backported that time because I planned to rewrite the
framebuffer initialization to use a common function for multiple
framebuffers.
As we now (due to your previous patches) have this logic it looks good
to me.
Thanks,
Florian Tobias Schandinat
> Signed-off-by: Jonathan Corbet <[email protected]>
> ---
> drivers/video/via/viafbdev.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
> index 91bfe6d..4a34d4f 100644
> --- a/drivers/video/via/viafbdev.c
> +++ b/drivers/video/via/viafbdev.c
> @@ -1991,7 +1991,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> if (!viafbinfo1) {
> printk(KERN_ERR
> "allocate the second framebuffer struct error\n");
> - goto out_delete_i2c;
> + goto out_unmap_screen;
> }
> viaparinfo1 = viafbinfo1->par;
> memcpy(viaparinfo1, viaparinfo, viafb_par_length);
> @@ -2086,6 +2086,8 @@ out_dealloc_cmap:
> out_fb1_release:
> if (viafbinfo1)
> framebuffer_release(viafbinfo1);
> +out_unmap_screen:
> + iounmap(viafbinfo->screen_base);
> out_delete_i2c:
> viafb_delete_i2c_buss(viaparinfo);
> out_fb_release:
Hi Jon,
Jonathan Corbet schrieb:
> Commit c3e25673843153ea75fda79a47cf12f10a25ca37 (viafb: 2D engine rewrite)
> changed the setting of the GEMODE register so that the reserved bits are no
> longer preserved. Fix that; at the same time, move this code to its own
> function and restore the use of symbolic constants.
in your later patch "[PATCH 06/16] viafb: complete support for
VX800/VX855 accelerated framebuffer" you reintroduce initializing those
bits to 0. That's fine but I can't see a reason for preserving this bits
here as it adds useless overhead unless the hardware itself changed some
of those bits and behaves differently according to those bits.
Additionally the first 2 bits are not reserved but provide a rotation
where 00 is what we want (no rotation).
And if you rip code off hw_bitblt_2 it would be better to do the same
with hw_bitblt_1. A quick look reveals that the same function can be
used there (the error message would need to be adjusted but that's minor).
So nack@preserving (unless it would cause problems otherwise) but
ack@ripping code off and sharing between hw_bitblt_1 & hw_bitblt_2.
Thanks,
Florian Tobias Schandinat
> Signed-off-by: Jonathan Corbet <[email protected]>
> ---
> drivers/video/via/accel.c | 48 ++++++++++++++++++++++++++++++---------------
> 1 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/via/accel.c b/drivers/video/via/accel.c
> index d5077df..78c0a2b 100644
> --- a/drivers/video/via/accel.c
> +++ b/drivers/video/via/accel.c
> @@ -165,12 +165,41 @@ static int hw_bitblt_1(void __iomem *engine, u8 op, u32 width, u32 height,
> return 0;
> }
>
> +/*
> + * Figure out an appropriate bytes-per-pixel setting.
> + */
> +static int viafb_set_bpp(void __iomem *engine, u8 bpp)
> +{
> + u32 gemode;
> +
> + /* Preserve the reserved bits */
> + gemode = readl(engine + VIA_REG_GEMODE) & 0xfffffcff;
> + switch (bpp) {
> + case 8:
> + gemode |= VIA_GEM_8bpp;
> + break;
> + case 16:
> + gemode |= VIA_GEM_16bpp;
> + break;
> + case 32:
> + gemode |= VIA_GEM_32bpp;
> + break;
> + default:
> + printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n", bpp);
> + return -EINVAL;
> + }
> + writel(gemode, engine + VIA_REG_GEMODE);
> + return 0;
> +}
> +
> +
> static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
> u8 dst_bpp, u32 dst_addr, u32 dst_pitch, u32 dst_x, u32 dst_y,
> u32 *src_mem, u32 src_addr, u32 src_pitch, u32 src_x, u32 src_y,
> u32 fg_color, u32 bg_color, u8 fill_rop)
> {
> u32 ge_cmd = 0, tmp, i;
> + int ret;
>
> if (!op || op > 3) {
> printk(KERN_WARNING "hw_bitblt_2: Invalid operation: %d\n", op);
> @@ -204,22 +233,9 @@ static int hw_bitblt_2(void __iomem *engine, u8 op, u32 width, u32 height,
> }
> }
>
> - switch (dst_bpp) {
> - case 8:
> - tmp = 0x00000000;
> - break;
> - case 16:
> - tmp = 0x00000100;
> - break;
> - case 32:
> - tmp = 0x00000300;
> - break;
> - default:
> - printk(KERN_WARNING "hw_bitblt_2: Unsupported bpp %d\n",
> - dst_bpp);
> - return -EINVAL;
> - }
> - writel(tmp, engine + 0x04);
> + ret = viafb_set_bpp(engine, dst_bpp);
> + if (ret)
> + return ret;
>
> if (op == VIA_BITBLT_FILL)
> tmp = 0;
Jonathan Corbet schrieb:
> From: Harald Welte <[email protected]>
>
> This will help us for the upcoming support for 2D acceleration using
> the M1 engine.
>
> [jc: fixed merge conflicts]
> Signed-off-by: Harald Welte <[email protected]>
That's probably a good idea (although most things are already done in
the separate blitting functions).
Just a minor nit:
Could we change the default so that if someone adds support for a new
IGP (and misses this function) we default to either the newest or
preferably to none? I've just seen too much poorly maintained code in
this driver and defaulting to the oldest is hence a bad idea.
Otherwise it's fine.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/via/chip.h | 8 ++++++++
> drivers/video/via/hw.c | 15 +++++++++++++++
> 2 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/via/chip.h b/drivers/video/via/chip.h
> index 474f428..f799e2c 100644
> --- a/drivers/video/via/chip.h
> +++ b/drivers/video/via/chip.h
> @@ -122,9 +122,17 @@ struct lvds_chip_information {
> int i2c_port;
> };
>
> +/* The type of 2D engine */
> +enum via_2d_engine {
> + VIA_2D_ENG_H2,
> + VIA_2D_ENG_H5,
> + VIA_2D_ENG_M1,
> +};
> +
> struct chip_information {
> int gfx_chip_name;
> int gfx_chip_revision;
> + enum via_2d_engine twod_engine;
> struct tmds_chip_information tmds_chip_info;
> struct lvds_chip_information lvds_chip_info;
> struct lvds_chip_information lvds_chip_info2;
> diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
> index f2e4bda..963704e 100644
> --- a/drivers/video/via/hw.c
> +++ b/drivers/video/via/hw.c
> @@ -2034,6 +2034,21 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
> CX700_REVISION_700;
> }
> }
> +
> + /* Determine which 2D engine we have */
> + switch (viaparinfo->chip_info->gfx_chip_name) {
> + case UNICHROME_VX800:
> + case UNICHROME_VX855:
> + viaparinfo->chip_info->twod_engine = VIA_2D_ENG_M1;
> + break;
> + case UNICHROME_K8M890:
> + case UNICHROME_P4M900:
> + viaparinfo->chip_info->twod_engine = VIA_2D_ENG_H5;
> + break;
> + default:
> + viaparinfo->chip_info->twod_engine = VIA_2D_ENG_H2;
> + break;
> + }
> }
>
> static void init_tmds_chip_info(void)
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 <[email protected]>
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
Hi Jon,
Jonathan Corbet schrieb:
> The following patches are the beginning of an effort to move work done for
> the OLPC XO 1.5 machine into the mainline. What's here is basic support
> for the VX855 chipset, I2C, suspend/resume, and the OLPC panel display.
Thanks a lot for your work I appreciate it very much.
However there are 2 things I'd like to ask for:
Could you please forward port them to latest mainline? There went some
changes in for 2.6.34 that break some of your patches (but not many and
at least the first conflict is easy to fix) as I generally prefer to
test a version that does not contain too much changes from myself.
Please make checkpatch happy where appropriate. (whitespaces, ...)
> What's coming in the future is a reworking of the viafb driver into
> something resembling a proper multifunction device, GPIO support, and
> camera support. But I'd like to start here.
Yeah, that's already a whole bunch of stuff. I agree that it's time to
get this in.
> If there's no objections, I'll start a tree and get it into linux-next in
> the near future, with an eye toward a 2.6.35 merge.
At least I'd like to see your work as early as possible so that we can
avoid conflicts (in patches and design). I hope this will get a bit
better after viafb is split up.
I don't have a strong opinion on the patch submission route as long as I
have the possibility to work on viafb and get some patches in as getting
to far ahead/away from mainline really sucks especially if one has not
all the hardware to test with. That said I'd be glad if we could
work/discuss the issues on these patches and get them in the next merge
window somehow.
Fortunately I still have a bit time left to finish the review of the
remaining patches in the next days but I would really appreciate it very
much if you could publish your work more frequently in the future.
Thanks,
Florian Tobias Schandinat
> Chris Ball (2):
> viafb: Add 1200x900 DCON/LCD panel modes for OLPC XO-1.5
> viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5
>
> Deepak Saxena (3):
> Minimal support for viafb suspend/resume
> VIAFB: Update suspend/resume to selectively restore registers
> Remove cursor restore hack in viafb
>
> Harald Welte (4):
> [FB] viafb: Fix various resource leaks during module_init()
> viafb: use proper pci config API
> viafb: Determine type of 2D engine and store it in chip_info
> viafb: rework the I2C support in the VIA framebuffer driver
>
> Jonathan Corbet (5):
> viafb: Unmap the frame buffer on initialization error
> viafb: Retain GEMODE reserved bits
> viafb: complete support for VX800/VX855 accelerated framebuffer
> viafb: rework suspend/resume
> viafb: Only suspend/resume on VX855
>
> Paul Fox (2):
> suppress verbose debug messages: change printk() to DEBUG_MSG()
> fix register save count, so it matches the restore count.
>
> drivers/video/via/accel.c | 87 ++++++++++++++-----
> drivers/video/via/accel.h | 40 +++++++++
> drivers/video/via/chip.h | 8 +
> drivers/video/via/dvi.c | 35 +++----
> drivers/video/via/hw.c | 84 +++++++++++++-----
> drivers/video/via/hw.h | 4
> drivers/video/via/ioctl.h | 2
> drivers/video/via/lcd.c | 29 ++++--
> drivers/video/via/lcd.h | 2
> drivers/video/via/share.h | 8 +
> drivers/video/via/via_i2c.c | 172 ++++++++++++++++++++++++--------------
> drivers/video/via/via_i2c.h | 43 ++++++---
> drivers/video/via/viafbdev.c | 191 +++++++++++++++++++++++++++++++++++++++----
> drivers/video/via/viafbdev.h | 5 -
> drivers/video/via/viamode.c | 14 +++
> drivers/video/via/vt1636.c | 36 +++-----
> drivers/video/via/vt1636.h | 2
> 17 files changed, 568 insertions(+), 194 deletions(-)
>
On Fri, 09 Apr 2010 07:43:35 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> Thanks a lot for your work I appreciate it very much.
> However there are 2 things I'd like to ask for:
>
> Could you please forward port them to latest mainline? There went some
> changes in for 2.6.34 that break some of your patches (but not many and
> at least the first conflict is easy to fix) as I generally prefer to
> test a version that does not contain too much changes from myself.
The OLPC tree is currently based on 2.6.34-rc1. I forgot that your
stuff went in after that; pulling things forward would make sense.
> Please make checkpatch happy where appropriate. (whitespaces, ...)
Hmm, will check again.
> Fortunately I still have a bit time left to finish the review of the
> remaining patches in the next days but I would really appreciate it very
> much if you could publish your work more frequently in the future.
Sorry, the only working platform I had was the OLPC 2.6.31 tree, and
there was little point in publishing that by the time it was ready to
be seen. That said, I *did* point out the tree where the work could be
seen for anybody who was interested.
In any case, it's very much my intent to get this stuff out there and
in for the next merge window.
Thanks for the reviews, I'll get to the specific issues in a bit.
jon
On Thu, 08 Apr 2010 20:22:57 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> > if (!viafbinfo->screen_base) {
> > printk(KERN_INFO "ioremap failed\n");
> > - return -ENOMEM;
> > + rc = -EIO;
>
> I don't know whether this is right (changing the return code) as Andrew
> recommend a while ago:
> "It should return -ENOMEM rather than -1, but that's minor."
> So I did and now I wonder which one is correct?
To me it seems like -ENOMEM could be a bit confusing here; there's a lot of
things that could go wrong with that same error return.
That said, I did some digging around, and -ENOMEM does seem to be the
standard response to an ioremap() failure. So I've changed it back.
> > if (!viafbinfo1) {
> > printk(KERN_ERR
> > "allocate the second framebuffer struct error\n");
> > - framebuffer_release(viafbinfo);
> > - return -ENOMEM;
>
> rc = -ENOMEM;
> is missing?
Indeed. Fixed.
Thanks,
jon
On Thu, 08 Apr 2010 20:42:17 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> something I am wondering about is whether we can't simply do:
> viaparinfo->memsize = pci_resource_len(pdev, 0);
> I suppose that this is not possible meaning that the pci len can be
> longer than the actual memory but I just wanted to use the moment to
> make sure.
That would make sense. But if somebody who is closer to the hardware than
I am doesn't take that approach, I'm nervous about changing it. Harald?
> This function was not designed to return an error (memsize is not
> checked). Either return a default value (let's say 8MB) or add a check
> for memsize.
That's a good point. I put in a check.
Thanks,
jon
On Fri, 09 Apr 2010 05:07:34 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> in your later patch "[PATCH 06/16] viafb: complete support for
> VX800/VX855 accelerated framebuffer" you reintroduce initializing those
> bits to 0. That's fine but I can't see a reason for preserving this bits
> here as it adds useless overhead unless the hardware itself changed some
> of those bits and behaves differently according to those bits.
Somehow the cost of an additional MMIO read at mode setting time is
just not going to keep me up at night.
I will admit that I've learned to be rather superstitious when it comes
to messing with reserved bits. Hardware designers like to hide
functionality like "bring down the wrath of the gods" behind such
bits. The old code preserved them and worked, so I did the same. I
don't see any real reason not to keep it.
> Additionally the first 2 bits are not reserved but provide a rotation
> where 00 is what we want (no rotation).
That much is true, yes. My mistake, will fix.
> And if you rip code off hw_bitblt_2 it would be better to do the same
> with hw_bitblt_1. A quick look reveals that the same function can be
> used there (the error message would need to be adjusted but that's minor).
That had crossed my mind; there is quite a bit of duplicated code
between those two very long functions. At the time I was focused on
making things work, and I didn't want to mess with code that I couldn't
actually test. So further cleanup is on my list, but I would prefer to
defer it for a little bit.
Thanks,
jon
On Fri, 09 Apr 2010 05:20:28 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> Just a minor nit:
> Could we change the default so that if someone adds support for a new
> IGP (and misses this function) we default to either the newest or
> preferably to none? I've just seen too much poorly maintained code in
> this driver and defaulting to the oldest is hence a bad idea.
> Otherwise it's fine.
That would require making an exhaustive list of older chipset types.
It could probably be inferred through inspection of the code, but I
worry about making assumptions in this area...
Thanks,
jon
On Fri, 09 Apr 2010 06:21:18 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> > + 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.
Ah, good point, the separate write is superfluous. Removed.
[...]
> > +#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.
Harald's initial patch included a mechanism for remapping register writes
on the fly depending on which engine was in use; these defines were used
for that purpose. Your reworking of the 2D code obliterated that patch,
and made it mostly unnecessary. I kept the defines around, though, because
I thought that documenting the registers can only be a good thing.
Thanks,
jon
Hi Jon,
Jonathan Corbet schrieb:
> On Fri, 09 Apr 2010 05:07:34 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
>> in your later patch "[PATCH 06/16] viafb: complete support for
>> VX800/VX855 accelerated framebuffer" you reintroduce initializing those
>> bits to 0. That's fine but I can't see a reason for preserving this bits
>> here as it adds useless overhead unless the hardware itself changed some
>> of those bits and behaves differently according to those bits.
>
> Somehow the cost of an additional MMIO read at mode setting time is
> just not going to keep me up at night.
Well it's not only mode setting its for each hardware accelerated
operation, but...
> I will admit that I've learned to be rather superstitious when it comes
> to messing with reserved bits. Hardware designers like to hide
> functionality like "bring down the wrath of the gods" behind such
> bits. The old code preserved them and worked, so I did the same. I
> don't see any real reason not to keep it.
...if you insist on it its okay with me. I still disagree but its
nothing I can't live with.
>> Additionally the first 2 bits are not reserved but provide a rotation
>> where 00 is what we want (no rotation).
>
> That much is true, yes. My mistake, will fix.
>
>> And if you rip code off hw_bitblt_2 it would be better to do the same
>> with hw_bitblt_1. A quick look reveals that the same function can be
>> used there (the error message would need to be adjusted but that's minor).
>
> That had crossed my mind; there is quite a bit of duplicated code
> between those two very long functions. At the time I was focused on
> making things work, and I didn't want to mess with code that I couldn't
> actually test. So further cleanup is on my list, but I would prefer to
> defer it for a little bit.
The code (and the spec regarding the reserved bits also) is obviously
identical so please don't ignore it. If you decide to put it in a
separate function please do so for both blit engines especially if they
really do the same as in this case. As you say they are mostly identical
and that's by design. Please keep them in sync if possible. They exist
that way to be a stateless and to avoid cluttering if's around.
(no need to say that I'll test those patches on my CLE266 and VX800 as
soon as they apply cleanly to mainline)
Thanks,
Florian Tobias Schandinat
On Fri, 09 Apr 2010 22:23:06 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> > That had crossed my mind; there is quite a bit of duplicated code
> > between those two very long functions. At the time I was focused on
> > making things work, and I didn't want to mess with code that I couldn't
> > actually test. So further cleanup is on my list, but I would prefer to
> > defer it for a little bit.
>
> The code (and the spec regarding the reserved bits also) is obviously
> identical so please don't ignore it.
In fact, I already came to this conclusion and have added a patch to
have both functions use the same code.
Thanks,
jon
Jonathan Corbet schrieb:
> On Fri, 09 Apr 2010 05:20:28 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
>> Just a minor nit:
>> Could we change the default so that if someone adds support for a new
>> IGP (and misses this function) we default to either the newest or
>> preferably to none? I've just seen too much poorly maintained code in
>> this driver and defaulting to the oldest is hence a bad idea.
>> Otherwise it's fine.
>
> That would require making an exhaustive list of older chipset types.
> It could probably be inferred through inspection of the code, but I
> worry about making assumptions in this area...
Such list already exists. gfx_chip_name = pdi->driver_data in hw.c (and
only there) so what is needed is the list viafb_pci_table in viafbdev.c
(relatively at the end) of all chips:
UNICHROME_CLE266
UNICHROME_PM800
UNICHROME_K400
UNICHROME_K800
UNICHROME_CN700
UNICHROME_K8M890
UNICHROME_CX700
UNICHROME_P4M900
UNICHROME_CN750
UNICHROME_VX800
UNICHROME_VX855
Would appreciate it if you could use this info.
Thanks,
Florian Tobias Schandinat
Jonathan Corbet schrieb:
> From: Chris Ball <[email protected]>
>
> [jc: extensive merge conflict fixes]
> Signed-off-by: Chris Ball <[email protected]>
> ---
> drivers/video/via/hw.c | 1 +
> drivers/video/via/ioctl.h | 2 +-
> drivers/video/via/lcd.c | 10 ++++++++++
> drivers/video/via/lcd.h | 2 ++
> drivers/video/via/share.h | 8 ++++++++
> drivers/video/via/viamode.c | 14 ++++++++++++++
> 6 files changed, 36 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
> index 963704e..7be462e 100644
> --- a/drivers/video/via/hw.c
> +++ b/drivers/video/via/hw.c
> @@ -63,6 +63,7 @@ static struct pll_map pll_value[] = {
> CX700_52_977M, VX855_52_977M},
> {CLK_56_250M, CLE266_PLL_56_250M, K800_PLL_56_250M,
> CX700_56_250M, VX855_56_250M},
> + {CLK_57_275M, 0, 0, 0, VX855_57_275M},
What will happen if someone with no VX855 will try to enter 1200x900
mode? Probably the world won't be destroyed but I have a really ugly
feeling that the driver is not well prepared for this situation. Haven't
tested yet as I'm waiting for the official forward port but I think it
might be better to reschedule this patch for later or add some PLL
values that are supposed to work (using/executing the formulas in the
VIA open source X driver)
Otherwise it looks fine.
Thanks,
Florian Tobias Schandinat
> {CLK_60_466M, CLE266_PLL_60_466M, K800_PLL_60_466M,
> CX700_60_466M, VX855_60_466M},
> {CLK_61_500M, CLE266_PLL_61_500M, K800_PLL_61_500M,
> diff --git a/drivers/video/via/ioctl.h b/drivers/video/via/ioctl.h
> index de89980..c430fa2 100644
> --- a/drivers/video/via/ioctl.h
> +++ b/drivers/video/via/ioctl.h
> @@ -75,7 +75,7 @@
> /*SAMM operation flag*/
> #define OP_SAMM 0x80
>
> -#define LCD_PANEL_ID_MAXIMUM 22
> +#define LCD_PANEL_ID_MAXIMUM 23
>
> #define STATE_ON 0x1
> #define STATE_OFF 0x0
> diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
> index 71e3200..e0e2310 100644
> --- a/drivers/video/via/lcd.c
> +++ b/drivers/video/via/lcd.c
> @@ -456,6 +456,16 @@ static int fp_id_to_vindex(int panel_id)
> viaparinfo->lvds_setting_info->LCDDithering = 1;
> return VIA_RES_480X640;
> break;
> + case 0x17:
> + /* OLPC XO-1.5 panel */
> + viaparinfo->lvds_setting_info->lcd_panel_hres = 1200;
> + viaparinfo->lvds_setting_info->lcd_panel_vres = 900;
> + viaparinfo->lvds_setting_info->lcd_panel_id =
> + LCD_PANEL_IDD_1200X900;
> + viaparinfo->lvds_setting_info->device_lcd_dualedge = 0;
> + viaparinfo->lvds_setting_info->LCDDithering = 0;
> + return VIA_RES_1200X900;
> + break;
> default:
> viaparinfo->lvds_setting_info->lcd_panel_hres = 800;
> viaparinfo->lvds_setting_info->lcd_panel_vres = 600;
> diff --git a/drivers/video/via/lcd.h b/drivers/video/via/lcd.h
> index 071f47c..9762ec6 100644
> --- a/drivers/video/via/lcd.h
> +++ b/drivers/video/via/lcd.h
> @@ -60,6 +60,8 @@
> #define LCD_PANEL_IDB_1360X768 0x0B
> /* Resolution: 480x640, Channel: single, Dithering: Enable */
> #define LCD_PANEL_IDC_480X640 0x0C
> +/* Resolution: 1200x900, Channel: single, Dithering: Disable */
> +#define LCD_PANEL_IDD_1200X900 0x0D
>
>
> extern int viafb_LCD2_ON;
> diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
> index 7cd03e2..ecbbf93 100644
> --- a/drivers/video/via/share.h
> +++ b/drivers/video/via/share.h
> @@ -86,6 +86,7 @@
> #define VIA_RES_1920X1200 38
> #define VIA_RES_2048X1536 39
> #define VIA_RES_480X640 40
> +#define VIA_RES_1200X900 41
>
> /*Reduce Blanking*/
> #define VIA_RES_1360X768_RB 131
> @@ -626,6 +627,10 @@
> #define M1200X720_R60_HSP NEGATIVE
> #define M1200X720_R60_VSP POSITIVE
>
> +/* 1200x900@60 Sync Polarity (DCON) */
> +#define M1200X900_R60_HSP NEGATIVE
> +#define M1200X900_R60_VSP NEGATIVE
> +
> /* 1280x600@60 Sync Polarity (GTF Mode) */
> #define M1280x600_R60_HSP NEGATIVE
> #define M1280x600_R60_VSP POSITIVE
> @@ -707,6 +712,7 @@
> #define CLK_52_406M 52406000
> #define CLK_52_977M 52977000
> #define CLK_56_250M 56250000
> +#define CLK_57_275M 57275000
> #define CLK_60_466M 60466000
> #define CLK_61_500M 61500000
> #define CLK_65_000M 65000000
> @@ -995,6 +1001,7 @@
> #define VX855_52_406M 0x00580C03
> #define VX855_52_977M 0x00940C05
> #define VX855_56_250M 0x009D0C05
> +#define VX855_57_275M 0x009D8C85 /* Used by XO panel */
> #define VX855_60_466M 0x00A90C05
> #define VX855_61_500M 0x00AC0C05
> #define VX855_65_000M 0x006D0C03
> @@ -1121,6 +1128,7 @@
> #define RES_1600X1200_60HZ_PIXCLOCK 6172
> #define RES_1600X1200_75HZ_PIXCLOCK 4938
> #define RES_1280X720_60HZ_PIXCLOCK 13426
> +#define RES_1200X900_60HZ_PIXCLOCK 17459
> #define RES_1920X1080_60HZ_PIXCLOCK 5787
> #define RES_1400X1050_60HZ_PIXCLOCK 8214
> #define RES_1400X1050_75HZ_PIXCLOCK 6410
> diff --git a/drivers/video/via/viamode.c b/drivers/video/via/viamode.c
> index b74f8a6..46833b4 100644
> --- a/drivers/video/via/viamode.c
> +++ b/drivers/video/via/viamode.c
> @@ -66,6 +66,7 @@ struct res_map_refresh res_map_refresh_tbl[] = {
> {1088, 612, RES_1088X612_60HZ_PIXCLOCK, 60},
> {1152, 720, RES_1152X720_60HZ_PIXCLOCK, 60},
> {1200, 720, RES_1200X720_60HZ_PIXCLOCK, 60},
> + {1200, 900, RES_1200X900_60HZ_PIXCLOCK, 60},
> {1280, 600, RES_1280X600_60HZ_PIXCLOCK, 60},
> {1280, 720, RES_1280X720_50HZ_PIXCLOCK, 50},
> {1280, 768, RES_1280X768_50HZ_PIXCLOCK, 50},
> @@ -759,6 +760,16 @@ struct crt_mode_table CRTM1200x720[] = {
> {1568, 1200, 1200, 368, 1256, 128, 746, 720, 720, 26, 721, 3} }
> };
>
> +/* 1200x900 (DCON) */
> +struct crt_mode_table DCON1200x900[] = {
> + /* r_rate, vclk, hsp, vsp */
> + {REFRESH_60, CLK_57_275M, M1200X900_R60_HSP, M1200X900_R60_VSP,
> + /* The correct htotal is 1240, but this doesn't raster on VX855. */
> + /* Via suggested changing to a multiple of 16, hence 1264. */
> + /* HT, HA, HBS, HBE, HSS, HSE, VT, VA, VBS, VBE, VSS, VSE */
> + {1264, 1200, 1200, 64, 1211, 32, 912, 900, 900, 12, 901, 10} }
> +};
> +
> /* 1280x600 (GTF) */
> struct crt_mode_table CRTM1280x600[] = {
> /* r_rate, vclk, hsp, vsp */
> @@ -946,6 +957,9 @@ struct VideoModeTable CLE266Modes[] = {
> /* Display : 1200x720 (GTF) */
> {VIA_RES_1200X720, CRTM1200x720, ARRAY_SIZE(CRTM1200x720)},
>
> + /* Display : 1200x900 (DCON) */
> + {VIA_RES_1200X900, DCON1200x900, ARRAY_SIZE(DCON1200x900)},
> +
> /* Display : 1280x600 (GTF) */
> {VIA_RES_1280X600, CRTM1280x600, ARRAY_SIZE(CRTM1280x600)},
>
Jonathan Corbet schrieb:
> From: Chris Ball <[email protected]>
>
> The i2c transactions involved in detecting LVDS (9 seconds) and TMDS
> (16 seconds) add an extra 25 seconds to viafb load time on the XO-1.5.
I don't like the idea of OLPC specific code. Isn't there any way to
speed this up in general?
There is not yet even an option for OLPC_XO_1_5 (in contrast to
CONFIG_OLPC) in mainline. Is such a thing planned?
I can't really see anything that would speak for accepting this patch
now in current mainline, sorry.
Thanks,
Florian Tobias Schandinat
> [jc: minor merge conflict fixed]
> Signed-off-by: Chris Ball <[email protected]>
> ---
> drivers/video/via/hw.c | 4 ++++
> drivers/video/via/lcd.c | 4 ++++
> 2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/via/hw.c b/drivers/video/via/hw.c
> index 7be462e..47ba09a 100644
> --- a/drivers/video/via/hw.c
> +++ b/drivers/video/via/hw.c
> @@ -2054,6 +2054,10 @@ static void init_gfx_chip_info(struct pci_dev *pdev,
>
> static void init_tmds_chip_info(void)
> {
> +#ifdef CONFIG_OLPC_XO_1_5
> + if (machine_is_olpc())
> + return;
> +#endif
> viafb_tmds_trasmitter_identify();
>
> if (INTERFACE_NONE == viaparinfo->chip_info->tmds_chip_info.
> diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
> index e0e2310..37a9746 100644
> --- a/drivers/video/via/lcd.c
> +++ b/drivers/video/via/lcd.c
> @@ -208,6 +208,10 @@ static bool lvds_identify_integratedlvds(void)
>
> int viafb_lvds_trasmitter_identify(void)
> {
> +#ifdef CONFIG_OLPC_XO_1_5
> + if (machine_is_olpc())
> + return FAIL;
> +#endif
> viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
> if (viafb_lvds_identify_vt1636()) {
> viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
Jonathan Corbet schrieb:
> From: Harald Welte <[email protected]>
>
> This patch changes the way how the various I2C busses are used internally
> inside the viafb driver: Previosuly, only a single i2c_adapter was created,
> even thougt two different hardware I2C busses are accessed: A structure member
> in a global variable was modified to indicate the bus to be used.
>
> Now, all existing hardware busses are registered with the i2c core, and the
> viafb_i2c_{read,write}byte[s]() function take the adapter number as function
> call parameter, rather than referring to the global structure member.
>
> [jc: even more painful merge with mainline changes ->2.6.34]
> [jc: painful merge with OLPC changes]
I don't know I2C too well and this patch has certainly its issues (the
FIXMEs, the hardcoded VIAFB_NUM_I2C, probably others) but in general it
looks okay. Therefore I think it should be merged and the issues should
be fixed afterwards.
Thanks,
Florian Tobias Schandinat
> Signed-off-by: Harald Welte <[email protected]>
> Signed-off-by: Jonathan Corbet <[email protected]>
> ---
> drivers/video/via/dvi.c | 35 ++++-----
> drivers/video/via/lcd.c | 15 ++--
> drivers/video/via/via_i2c.c | 172 ++++++++++++++++++++++++++----------------
> drivers/video/via/via_i2c.h | 43 +++++++----
> drivers/video/via/viafbdev.c | 6 +-
> drivers/video/via/viafbdev.h | 4 +-
> drivers/video/via/vt1636.c | 36 ++++-----
> drivers/video/via/vt1636.h | 2 +-
> 8 files changed, 181 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/video/via/dvi.c b/drivers/video/via/dvi.c
> index 67b3693..00d001e 100644
> --- a/drivers/video/via/dvi.c
> +++ b/drivers/video/via/dvi.c
> @@ -96,7 +96,7 @@ int viafb_tmds_trasmitter_identify(void)
> viaparinfo->chip_info->tmds_chip_info.tmds_chip_name = VT1632_TMDS;
> viaparinfo->chip_info->
> tmds_chip_info.tmds_chip_slave_addr = VT1632_TMDS_I2C_ADDR;
> - viaparinfo->chip_info->tmds_chip_info.i2c_port = I2CPORTINDEX;
> + viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_31;
> if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID) != FAIL) {
> /*
> * Currently only support 12bits,dual edge,add 24bits mode later
> @@ -110,7 +110,7 @@ int viafb_tmds_trasmitter_identify(void)
> viaparinfo->chip_info->tmds_chip_info.i2c_port);
> return OK;
> } else {
> - viaparinfo->chip_info->tmds_chip_info.i2c_port = GPIOPORTINDEX;
> + viaparinfo->chip_info->tmds_chip_info.i2c_port = VIA_I2C_ADAP_2C;
> if (check_tmds_chip(VT1632_DEVICE_ID_REG, VT1632_DEVICE_ID)
> != FAIL) {
> tmds_register_write(0x08, 0x3b);
> @@ -160,32 +160,26 @@ int viafb_tmds_trasmitter_identify(void)
>
> static void tmds_register_write(int index, u8 data)
> {
> - viaparinfo->shared->i2c_stuff.i2c_port =
> - viaparinfo->chip_info->tmds_chip_info.i2c_port;
> -
> - viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.
> - tmds_chip_slave_addr, index,
> - data);
> + viafb_i2c_writebyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
> + viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
> + index, data);
> }
>
> static int tmds_register_read(int index)
> {
> u8 data;
>
> - viaparinfo->shared->i2c_stuff.i2c_port =
> - viaparinfo->chip_info->tmds_chip_info.i2c_port;
> - viafb_i2c_readbyte((u8) viaparinfo->chip_info->
> - tmds_chip_info.tmds_chip_slave_addr,
> - (u8) index, &data);
> + viafb_i2c_readbyte(viaparinfo->chip_info->tmds_chip_info.i2c_port,
> + (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
> + (u8) index, &data);
> return data;
> }
>
> static int tmds_register_read_bytes(int index, u8 *buff, int buff_len)
> {
> - viaparinfo->shared->i2c_stuff.i2c_port =
> - viaparinfo->chip_info->tmds_chip_info.i2c_port;
> - viafb_i2c_readbytes((u8) viaparinfo->chip_info->tmds_chip_info.
> - tmds_chip_slave_addr, (u8) index, buff, buff_len);
> + viafb_i2c_readbytes(viaparinfo->chip_info->tmds_chip_info.i2c_port,
> + (u8) viaparinfo->chip_info->tmds_chip_info.tmds_chip_slave_addr,
> + (u8) index, buff, buff_len);
> return 0;
> }
>
> @@ -648,9 +642,10 @@ void viafb_dvi_enable(void)
> else
> data = 0x37;
> viafb_i2c_writebyte(viaparinfo->chip_info->
> - tmds_chip_info.
> - tmds_chip_slave_addr,
> - 0x08, data);
> + tmds_chip_info.i2c_port,
> + viaparinfo->chip_info->
> + tmds_chip_info.tmds_chip_slave_addr,
> + 0x08, data);
> }
> }
> }
> diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
> index 37a9746..fc27534 100644
> --- a/drivers/video/via/lcd.c
> +++ b/drivers/video/via/lcd.c
> @@ -212,16 +212,14 @@ int viafb_lvds_trasmitter_identify(void)
> if (machine_is_olpc())
> return FAIL;
> #endif
> - viaparinfo->shared->i2c_stuff.i2c_port = I2CPORTINDEX;
> - if (viafb_lvds_identify_vt1636()) {
> - viaparinfo->chip_info->lvds_chip_info.i2c_port = I2CPORTINDEX;
> + if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_31)) {
> + viaparinfo->chip_info->lvds_chip_info.i2c_port = VIA_I2C_ADAP_31;
> DEBUG_MSG(KERN_INFO
> "Found VIA VT1636 LVDS on port i2c 0x31 \n");
> } else {
> - viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
> - if (viafb_lvds_identify_vt1636()) {
> + if (viafb_lvds_identify_vt1636(VIA_I2C_ADAP_26)) {
> viaparinfo->chip_info->lvds_chip_info.i2c_port =
> - GPIOPORTINDEX;
> + VIA_I2C_ADAP_2C;
> DEBUG_MSG(KERN_INFO
> "Found VIA VT1636 LVDS on port gpio 0x2c \n");
> }
> @@ -485,9 +483,8 @@ static int lvds_register_read(int index)
> {
> u8 data;
>
> - viaparinfo->shared->i2c_stuff.i2c_port = GPIOPORTINDEX;
> - viafb_i2c_readbyte((u8) viaparinfo->chip_info->
> - lvds_chip_info.lvds_chip_slave_addr,
> + viafb_i2c_readbyte(VIA_I2C_ADAP_2C,
> + (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> (u8) index, &data);
> return data;
> }
> diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
> index 15543e9..18bbbdd 100644
> --- a/drivers/video/via/via_i2c.c
> +++ b/drivers/video/via/via_i2c.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
> + * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
> * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
>
> * This program is free software; you can redistribute it and/or
> @@ -24,40 +24,44 @@
> static void via_i2c_setscl(void *data, int state)
> {
> u8 val;
> - struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> + struct via_i2c_adap_cfg *adap_data = data;
>
> - val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
> + printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
> + adap_data->ioport_index, adap_data->io_port);
> + val = viafb_read_reg(adap_data->io_port,
> + adap_data->ioport_index) & 0xF0;
> if (state)
> val |= 0x20;
> else
> val &= ~0x20;
> - switch (via_i2c_chan->i2c_port) {
> - case I2CPORTINDEX:
> + switch (adap_data->type) {
> + case VIA_I2C_I2C:
> val |= 0x01;
> break;
> - case GPIOPORTINDEX:
> + case VIA_I2C_GPIO:
> val |= 0x80;
> break;
> default:
> - DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
> + DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
> }
> - viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
> + viafb_write_reg(adap_data->ioport_index,
> + adap_data->io_port, val);
> }
>
> static int via_i2c_getscl(void *data)
> {
> - struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> + struct via_i2c_adap_cfg *adap_data = data;
>
> - if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x08)
> + if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x08)
> return 1;
> return 0;
> }
>
> static int via_i2c_getsda(void *data)
> {
> - struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> + struct via_i2c_adap_cfg *adap_data = data;
>
> - if (viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0x04)
> + if (viafb_read_reg(adap_data->io_port, adap_data->ioport_index) & 0x04)
> return 1;
> return 0;
> }
> @@ -65,27 +69,29 @@ static int via_i2c_getsda(void *data)
> static void via_i2c_setsda(void *data, int state)
> {
> u8 val;
> - struct via_i2c_stuff *via_i2c_chan = (struct via_i2c_stuff *)data;
> + struct via_i2c_adap_cfg *adap_data = data;
>
> - val = viafb_read_reg(VIASR, via_i2c_chan->i2c_port) & 0xF0;
> + val = viafb_read_reg(adap_data->io_port,
> + adap_data->ioport_index) & 0xF0;
> if (state)
> val |= 0x10;
> else
> val &= ~0x10;
> - switch (via_i2c_chan->i2c_port) {
> - case I2CPORTINDEX:
> + switch (adap_data->type) {
> + case VIA_I2C_I2C:
> val |= 0x01;
> break;
> - case GPIOPORTINDEX:
> + case VIA_I2C_GPIO:
> val |= 0x40;
> break;
> default:
> - DEBUG_MSG("via_i2c: specify wrong i2c port.\n");
> + DEBUG_MSG("viafb_i2c: specify wrong i2c type.\n");
> }
> - viafb_write_reg(via_i2c_chan->i2c_port, VIASR, val);
> + viafb_write_reg(adap_data->ioport_index,
> + adap_data->io_port, val);
> }
>
> -int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
> +int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata)
> {
> u8 mm1[] = {0x00};
> struct i2c_msg msgs[2];
> @@ -97,12 +103,11 @@ int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata)
> mm1[0] = index;
> msgs[0].len = 1; msgs[1].len = 1;
> msgs[0].buf = mm1; msgs[1].buf = pdata;
> - i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
> -
> - return 0;
> + return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
> + msgs, 2);
> }
>
> -int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
> +int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data)
> {
> u8 msg[2] = { index, data };
> struct i2c_msg msgs;
> @@ -111,10 +116,11 @@ int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data)
> msgs.addr = slave_addr / 2;
> msgs.len = 2;
> msgs.buf = msg;
> - return i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, &msgs, 1);
> + return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
> + &msgs, 1);
> }
>
> -int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
> +int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len)
> {
> u8 mm1[] = {0x00};
> struct i2c_msg msgs[2];
> @@ -125,53 +131,89 @@ int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len)
> mm1[0] = index;
> msgs[0].len = 1; msgs[1].len = buff_len;
> msgs[0].buf = mm1; msgs[1].buf = buff;
> - i2c_transfer(&viaparinfo->shared->i2c_stuff.adapter, msgs, 2);
> - return 0;
> + return i2c_transfer(&viaparinfo->shared->i2c_stuff[adap].adapter,
> + msgs, 2);
> }
>
> -int viafb_create_i2c_bus(void *viapar)
> +static int create_i2c_bus(struct i2c_adapter *adapter,
> + struct i2c_algo_bit_data *algo,
> + struct via_i2c_adap_cfg *adap_cfg,
> + struct pci_dev *pdev)
> {
> - int ret;
> - struct via_i2c_stuff *i2c_stuff =
> - &((struct viafb_par *)viapar)->shared->i2c_stuff;
> -
> - strcpy(i2c_stuff->adapter.name, "via_i2c");
> - i2c_stuff->i2c_port = 0x0;
> - i2c_stuff->adapter.owner = THIS_MODULE;
> - i2c_stuff->adapter.id = 0x01FFFF;
> - i2c_stuff->adapter.class = 0;
> - i2c_stuff->adapter.algo_data = &i2c_stuff->algo;
> - i2c_stuff->adapter.dev.parent = NULL;
> - i2c_stuff->algo.setsda = via_i2c_setsda;
> - i2c_stuff->algo.setscl = via_i2c_setscl;
> - i2c_stuff->algo.getsda = via_i2c_getsda;
> - i2c_stuff->algo.getscl = via_i2c_getscl;
> - i2c_stuff->algo.udelay = 40;
> - i2c_stuff->algo.timeout = 20;
> - i2c_stuff->algo.data = i2c_stuff;
> -
> - i2c_set_adapdata(&i2c_stuff->adapter, i2c_stuff);
> + printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
> +
> + algo->setsda = via_i2c_setsda;
> + algo->setscl = via_i2c_setscl;
> + algo->getsda = via_i2c_getsda;
> + algo->getscl = via_i2c_getscl;
> + algo->udelay = 40;
> + algo->timeout = 20;
> + algo->data = adap_cfg;
> +
> + sprintf(adapter->name, "viafb i2c io_port idx 0x%02x",
> + adap_cfg->ioport_index);
> + adapter->owner = THIS_MODULE;
> + adapter->id = 0x01FFFF;
> + adapter->class = I2C_CLASS_DDC;
> + adapter->algo_data = algo;
> + if (pdev)
> + adapter->dev.parent = &pdev->dev;
> + else
> + adapter->dev.parent = NULL;
> + //i2c_set_adapdata(adapter, adap_cfg);
>
> /* Raise SCL and SDA */
> - i2c_stuff->i2c_port = I2CPORTINDEX;
> - via_i2c_setsda(i2c_stuff, 1);
> - via_i2c_setscl(i2c_stuff, 1);
> -
> - i2c_stuff->i2c_port = GPIOPORTINDEX;
> - via_i2c_setsda(i2c_stuff, 1);
> - via_i2c_setscl(i2c_stuff, 1);
> + via_i2c_setsda(adap_cfg, 1);
> + via_i2c_setscl(adap_cfg, 1);
> udelay(20);
>
> - ret = i2c_bit_add_bus(&i2c_stuff->adapter);
> - if (ret == 0)
> - DEBUG_MSG("I2C bus %s registered.\n", i2c_stuff->adapter.name);
> - else
> - DEBUG_MSG("Failed to register I2C bus %s.\n",
> - i2c_stuff->adapter.name);
> - return ret;
> + return i2c_bit_add_bus(adapter);
> +}
> +
> +static struct via_i2c_adap_cfg adap_configs[] = {
> + [VIA_I2C_ADAP_26] = { VIA_I2C_I2C, VIASR, 0x26 },
> + [VIA_I2C_ADAP_31] = { VIA_I2C_I2C, VIASR, 0x31 },
> + [VIA_I2C_ADAP_25] = { VIA_I2C_GPIO, VIASR, 0x25 },
> + [VIA_I2C_ADAP_2C] = { VIA_I2C_GPIO, VIASR, 0x2c },
> + [VIA_I2C_ADAP_3D] = { VIA_I2C_GPIO, VIASR, 0x3d },
> + { 0, 0, 0 }
> +};
> +
> +int viafb_create_i2c_busses(struct viafb_par *viapar)
> +{
> + int i, ret;
> + printk(KERN_DEBUG "%s: entering\n", __FUNCTION__);
> +
> + for (i = 0; i < VIAFB_NUM_I2C; i++) {
> + struct via_i2c_adap_cfg *adap_cfg = &adap_configs[i];
> + struct via_i2c_stuff *i2c_stuff = &viapar->shared->i2c_stuff[i];
> +
> + if (adap_cfg->type == 0)
> + break;
> +
> + ret = create_i2c_bus(&i2c_stuff->adapter,
> + &i2c_stuff->algo, adap_cfg,
> + NULL); //FIXME: PCIDEV
> + if (ret < 0) {
> + printk(KERN_ERR "viafb: cannot create i2c bus %u:%d\n",
> + i, ret);
> + //FIXME: properly release previous busses
> + return ret;
> + }
> + }
> +
> + return 0;
> }
>
> -void viafb_delete_i2c_buss(void *par)
> +void viafb_delete_i2c_busses(struct viafb_par *par)
> {
> - i2c_del_adapter(&((struct viafb_par *)par)->shared->i2c_stuff.adapter);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(par->shared->i2c_stuff); i++) {
> + struct via_i2c_stuff *i2c_stuff = &par->shared->i2c_stuff[i];
> + /* only remove those entries in the array that we've
> + * actually used (and thus initialized algo_data) */
> + if (i2c_stuff->adapter.algo_data == &i2c_stuff->algo)
> + i2c_del_adapter(&i2c_stuff->adapter);
> + }
> }
> diff --git a/drivers/video/via/via_i2c.h b/drivers/video/via/via_i2c.h
> index 3a13242..00ed978 100644
> --- a/drivers/video/via/via_i2c.h
> +++ b/drivers/video/via/via_i2c.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright 1998-2008 VIA Technologies, Inc. All Rights Reserved.
> + * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
> * Copyright 2001-2008 S3 Graphics, Inc. All Rights Reserved.
>
> * This program is free software; you can redistribute it and/or
> @@ -24,23 +24,38 @@
> #include <linux/i2c.h>
> #include <linux/i2c-algo-bit.h>
>
> +enum via_i2c_type {
> + VIA_I2C_NONE,
> + VIA_I2C_I2C,
> + VIA_I2C_GPIO,
> +};
> +
> +/* private data for each adapter */
> +struct via_i2c_adap_cfg {
> + enum via_i2c_type type;
> + u_int16_t io_port;
> + u_int8_t ioport_index;
> +};
> +
> struct via_i2c_stuff {
> u16 i2c_port; /* GPIO or I2C port */
> struct i2c_adapter adapter;
> struct i2c_algo_bit_data algo;
> };
>
> -#define I2CPORT 0x3c4
> -#define I2CPORTINDEX 0x31
> -#define GPIOPORT 0x3C4
> -#define GPIOPORTINDEX 0x2C
> -#define I2C_BUS 1
> -#define GPIO_BUS 2
> -#define DELAYPORT 0x3C3
> -
> -int viafb_i2c_readbyte(u8 slave_addr, u8 index, u8 *pdata);
> -int viafb_i2c_writebyte(u8 slave_addr, u8 index, u8 data);
> -int viafb_i2c_readbytes(u8 slave_addr, u8 index, u8 *buff, int buff_len);
> -int viafb_create_i2c_bus(void *par);
> -void viafb_delete_i2c_buss(void *par);
> +enum viafb_i2c_adap {
> + VIA_I2C_ADAP_26,
> + VIA_I2C_ADAP_31,
> + VIA_I2C_ADAP_25,
> + VIA_I2C_ADAP_2C,
> + VIA_I2C_ADAP_3D,
> +};
> +
> +int viafb_i2c_readbyte(u8 adap, u8 slave_addr, u8 index, u8 *pdata);
> +int viafb_i2c_writebyte(u8 adap, u8 slave_addr, u8 index, u8 data);
> +int viafb_i2c_readbytes(u8 adap, u8 slave_addr, u8 index, u8 *buff, int buff_len);
> +
> +struct viafb_par;
> +int viafb_create_i2c_busses(struct viafb_par *par);
> +void viafb_delete_i2c_busses(struct viafb_par *par);
> #endif /* __VIA_I2C_H__ */
> diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
> index 4a34d4f..ea3018e 100644
> --- a/drivers/video/via/viafbdev.c
> +++ b/drivers/video/via/viafbdev.c
> @@ -1886,7 +1886,7 @@ static int __devinit via_pci_probe(struct pci_dev *pdev,
> viafb_dual_fb = 0;
>
> /* Set up I2C bus stuff */
> - rc = viafb_create_i2c_bus(viaparinfo);
> + rc = viafb_create_i2c_busses(viaparinfo);
> if (rc)
> goto out_fb_release;
>
> @@ -2089,7 +2089,7 @@ out_fb1_release:
> out_unmap_screen:
> iounmap(viafbinfo->screen_base);
> out_delete_i2c:
> - viafb_delete_i2c_buss(viaparinfo);
> + viafb_delete_i2c_busses(viaparinfo);
> out_fb_release:
> framebuffer_release(viafbinfo);
> return rc;
> @@ -2105,7 +2105,7 @@ static void __devexit via_pci_remove(struct pci_dev *pdev)
> iounmap((void *)viafbinfo->screen_base);
> iounmap(viaparinfo->shared->engine_mmio);
>
> - viafb_delete_i2c_buss(viaparinfo);
> + viafb_delete_i2c_busses(viaparinfo);
>
> framebuffer_release(viafbinfo);
> if (viafb_dual_fb)
> diff --git a/drivers/video/via/viafbdev.h b/drivers/video/via/viafbdev.h
> index 0c94d24..1dab97b 100644
> --- a/drivers/video/via/viafbdev.h
> +++ b/drivers/video/via/viafbdev.h
> @@ -37,11 +37,13 @@
> #define VERSION_OS 0 /* 0: for 32 bits OS, 1: for 64 bits OS */
> #define VERSION_MINOR 4
>
> +#define VIAFB_NUM_I2C 5
> +
> struct viafb_shared {
> struct proc_dir_entry *proc_entry; /*viafb proc entry */
>
> /* I2C stuff */
> - struct via_i2c_stuff i2c_stuff;
> + struct via_i2c_stuff i2c_stuff[VIAFB_NUM_I2C];
>
> /* All the information will be needed to set engine */
> struct tmds_setting_information tmds_setting_info;
> diff --git a/drivers/video/via/vt1636.c b/drivers/video/via/vt1636.c
> index a6b3749..d75b5f0 100644
> --- a/drivers/video/via/vt1636.c
> +++ b/drivers/video/via/vt1636.c
> @@ -27,9 +27,8 @@ u8 viafb_gpio_i2c_read_lvds(struct lvds_setting_information
> {
> u8 data;
>
> - viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
> - viafb_i2c_readbyte(plvds_chip_info->lvds_chip_slave_addr, index, &data);
> -
> + viafb_i2c_readbyte(plvds_chip_info->i2c_port,
> + plvds_chip_info->lvds_chip_slave_addr, index, &data);
> return data;
> }
>
> @@ -39,14 +38,13 @@ void viafb_gpio_i2c_write_mask_lvds(struct lvds_setting_information
> {
> int index, data;
>
> - viaparinfo->shared->i2c_stuff.i2c_port = plvds_chip_info->i2c_port;
> -
> index = io_data.Index;
> data = viafb_gpio_i2c_read_lvds(plvds_setting_info, plvds_chip_info,
> index);
> data = (data & (~io_data.Mask)) | io_data.Data;
>
> - viafb_i2c_writebyte(plvds_chip_info->lvds_chip_slave_addr, index, data);
> + viafb_i2c_writebyte(plvds_chip_info->i2c_port,
> + plvds_chip_info->lvds_chip_slave_addr, index, data);
> }
>
> void viafb_init_lvds_vt1636(struct lvds_setting_information
> @@ -159,7 +157,7 @@ void viafb_disable_lvds_vt1636(struct lvds_setting_information
> }
> }
>
> -bool viafb_lvds_identify_vt1636(void)
> +bool viafb_lvds_identify_vt1636(u8 i2c_adapter)
> {
> u8 Buffer[2];
>
> @@ -170,23 +168,23 @@ bool viafb_lvds_identify_vt1636(void)
> VT1636_LVDS_I2C_ADDR;
>
> /* Check vendor ID first: */
> - viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> - lvds_chip_slave_addr,
> - 0x00, &Buffer[0]);
> - viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> - lvds_chip_slave_addr,
> - 0x01, &Buffer[1]);
> + viafb_i2c_readbyte(i2c_adapter,
> + (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> + 0x00, &Buffer[0]);
> + viafb_i2c_readbyte(i2c_adapter,
> + (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> + 0x01, &Buffer[1]);
>
> if (!((Buffer[0] == 0x06) && (Buffer[1] == 0x11)))
> return false;
>
> /* Check Chip ID: */
> - viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> - lvds_chip_slave_addr,
> - 0x02, &Buffer[0]);
> - viafb_i2c_readbyte((u8) viaparinfo->chip_info->lvds_chip_info.
> - lvds_chip_slave_addr,
> - 0x03, &Buffer[1]);
> + viafb_i2c_readbyte(i2c_adapter,
> + (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> + 0x02, &Buffer[0]);
> + viafb_i2c_readbyte(i2c_adapter,
> + (u8) viaparinfo->chip_info->lvds_chip_info.lvds_chip_slave_addr,
> + 0x03, &Buffer[1]);
> if ((Buffer[0] == 0x45) && (Buffer[1] == 0x33)) {
> viaparinfo->chip_info->lvds_chip_info.lvds_chip_name =
> VT1636_LVDS;
> diff --git a/drivers/video/via/vt1636.h b/drivers/video/via/vt1636.h
> index 2a150c5..4c1314e 100644
> --- a/drivers/video/via/vt1636.h
> +++ b/drivers/video/via/vt1636.h
> @@ -22,7 +22,7 @@
> #ifndef _VT1636_H_
> #define _VT1636_H_
> #include "chip.h"
> -bool viafb_lvds_identify_vt1636(void);
> +bool viafb_lvds_identify_vt1636(u8 i2c_adapter);
> void viafb_init_lvds_vt1636(struct lvds_setting_information
> *plvds_setting_info, struct lvds_chip_information *plvds_chip_info);
> void viafb_enable_lvds_vt1636(struct lvds_setting_information
This patch is fine.
Thanks,
Florian Tobias Schandinat
Jonathan Corbet schrieb:
> From: Paul Fox <[email protected]>
>
> ---
> drivers/video/via/via_i2c.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/via/via_i2c.c b/drivers/video/via/via_i2c.c
> index 18bbbdd..86e6e8d 100644
> --- a/drivers/video/via/via_i2c.c
> +++ b/drivers/video/via/via_i2c.c
> @@ -26,7 +26,7 @@ static void via_i2c_setscl(void *data, int state)
> u8 val;
> struct via_i2c_adap_cfg *adap_data = data;
>
> - printk(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
> + DEBUG_MSG(KERN_DEBUG "reading index 0x%02x from IO 0x%x\n",
> adap_data->ioport_index, adap_data->io_port);
> val = viafb_read_reg(adap_data->io_port,
> adap_data->ioport_index) & 0xF0;
> @@ -140,7 +140,7 @@ static int create_i2c_bus(struct i2c_adapter *adapter,
> struct via_i2c_adap_cfg *adap_cfg,
> struct pci_dev *pdev)
> {
> - printk(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
> + DEBUG_MSG(KERN_DEBUG "viafb: creating bus adap=0x%p, algo_bit_data=0x%p, adap_cfg=0x%p\n", adapter, algo, adap_cfg);
>
> algo->setsda = via_i2c_setsda;
> algo->setscl = via_i2c_setscl;
> @@ -182,7 +182,7 @@ static struct via_i2c_adap_cfg adap_configs[] = {
> int viafb_create_i2c_busses(struct viafb_par *viapar)
> {
> int i, ret;
> - printk(KERN_DEBUG "%s: entering\n", __FUNCTION__);
> + DEBUG_MSG(KERN_DEBUG "%s: entering\n", __FUNCTION__);
>
> for (i = 0; i < VIAFB_NUM_I2C; i++) {
> struct via_i2c_adap_cfg *adap_cfg = &adap_configs[i];
Jonathan Corbet schrieb:
> Sorry, the only working platform I had was the OLPC 2.6.31 tree, and
> there was little point in publishing that by the time it was ready to
> be seen. That said, I *did* point out the tree where the work could be
> seen for anybody who was interested.
Well the time I looked in your tree I didn't see any of the remaining
suspend/resume efforts. Okay perhaps I should have rechecked it now and
than.
Please correct me if I am wrong but the remaining 6 patches concerning
suspend&resume look like a real big FIXME. So at the end it is expected
to work only on VX855 and needs something called OFW?
It doesn't seem to make much sense to review each of them because the
following patches might or might not correct some of the issues of the
other. It is really a pain to have 6 patches trying to add a single
feature. Is there any way to fix this mess. (I assume you didn't merge
them due to authorship issues?)
I think it might be better to drop those for now and wait for viafb to
be in a better shape before adding this feature. The mode setting should
be in a pretty good shape just 1 or 2 kernel versions ahead so that the
dependency on OFW can be dropped I think.
Sorry but I really think this is not in a shape where merging it is an
option. I think it would be better to skip those suspend/resume patches
for the next merge window.
Thanks,
Florian Tobias Schandinat
On Fri, 09 Apr 2010 23:40:55 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> I don't like the idea of OLPC specific code. Isn't there any way to
> speed this up in general?
Architecture-specific code happens. OLPCs are wired differently; if
you go trying to do LVDS out those GPIO ports on an OLPC, you'll not
end up talking to the hardware you think you're talking to. The best
thing to do is to avoid it altogether.
> There is not yet even an option for OLPC_XO_1_5 (in contrast to
> CONFIG_OLPC) in mainline. Is such a thing planned?
Yes, it is. That's part of the remaining OLPC support code which has
also been brought forward to 2.6.34 with the intention of mainlining it.
> I can't really see anything that would speak for accepting this patch
> now in current mainline, sorry.
If you can come up with a better solution to the problem, I'm all
ears. But without it you'll have a hard time running mainline kernels
on XO 1.5 systems. It is all coming, but the OLPC folks are scrambling
to get everything together; I don't think we really need to make things
harder for them.
That said, machine_is_olpc() is properly defined for all
configurations, so the #ifdefs can (and should) come out.
jon
On Sat, 10 Apr 2010 01:32:36 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> Please correct me if I am wrong but the remaining 6 patches concerning
> suspend&resume look like a real big FIXME. So at the end it is expected
> to work only on VX855 and needs something called OFW?
OFW = OpenFirmware. You could say BIOS instead. It's pretty normal to
expect the BIOS to put things into a semi-rational state at resume time.
> It doesn't seem to make much sense to review each of them because the
> following patches might or might not correct some of the issues of the
> other. It is really a pain to have 6 patches trying to add a single
> feature. Is there any way to fix this mess. (I assume you didn't merge
> them due to authorship issues?)
It's true that one needs to look at the end product. I only looked at
the S/R code recently, and tried to fix some of the biggest issues that
would keep it out of the mainline.
> I think it might be better to drop those for now and wait for viafb to
> be in a better shape before adding this feature. The mode setting should
> be in a pretty good shape just 1 or 2 kernel versions ahead so that the
> dependency on OFW can be dropped I think.
>
> Sorry but I really think this is not in a shape where merging it is an
> option. I think it would be better to skip those suspend/resume patches
> for the next merge window.
Well, if we want to keep s/r out of tree, we can do that. It will
complicate the merge of the other stuff, since it's got hooks into the
GPIO and camera code too. But, like everything else I've posted so
far, it's not the work that I personally set out to do. I can push
that work on others :)
That said, the suspend/resume support in this patch set makes suspend
work on one chipset, and probably comes pretty close on the others
without breaking anything there. I don't see the harm in merging it;
it makes the code better than it is now. I would rather not have to
separate it out from the rest. But I'll not fight over this one; if
there's real opposition then we can force OLPC to continue to carry it
out of tree.
jon
Jonathan Corbet schrieb:
> On Fri, 09 Apr 2010 23:40:55 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
>> I don't like the idea of OLPC specific code. Isn't there any way to
>> speed this up in general?
>
> Architecture-specific code happens. OLPCs are wired differently; if
> you go trying to do LVDS out those GPIO ports on an OLPC, you'll not
> end up talking to the hardware you think you're talking to. The best
> thing to do is to avoid it altogether.
*sigh* I feared it would be something like this.
>> There is not yet even an option for OLPC_XO_1_5 (in contrast to
>> CONFIG_OLPC) in mainline. Is such a thing planned?
>
> Yes, it is. That's part of the remaining OLPC support code which has
> also been brought forward to 2.6.34 with the intention of mainlining it.
>
>> I can't really see anything that would speak for accepting this patch
>> now in current mainline, sorry.
>
> If you can come up with a better solution to the problem, I'm all
> ears. But without it you'll have a hard time running mainline kernels
> on XO 1.5 systems. It is all coming, but the OLPC folks are scrambling
> to get everything together; I don't think we really need to make things
> harder for them.
Sadly no as you probably know the OLPC hardware much better than me.
However I do not intend to give the OLPC folks a hard time.
> That said, machine_is_olpc() is properly defined for all
> configurations, so the #ifdefs can (and should) come out.
I'm not sure I get you right here. If you talk about removing the
defines and only letting the machine check that is something that I
would accept now. If this is not what you meant I think it would be
better to move this patch to the series adding the config option.
Thanks,
Florian Tobias Schandinat
On Sat, 10 Apr 2010 02:42:52 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> > That said, machine_is_olpc() is properly defined for all
> > configurations, so the #ifdefs can (and should) come out.
>
> I'm not sure I get you right here. If you talk about removing the
> defines and only letting the machine check that is something that I
> would accept now.
Yes, that is what I mean; the ifdefs don't need to be there. Had I
thought that through I would have removed them before posting the patch.
jon
Jonathan Corbet schrieb:
> On Sat, 10 Apr 2010 01:32:36 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
>> Please correct me if I am wrong but the remaining 6 patches concerning
>> suspend&resume look like a real big FIXME. So at the end it is expected
>> to work only on VX855 and needs something called OFW?
>
> OFW = OpenFirmware. You could say BIOS instead. It's pretty normal to
> expect the BIOS to put things into a semi-rational state at resume time.
So its more or less the same I always do for testing the module.
> Well, if we want to keep s/r out of tree, we can do that. It will
> complicate the merge of the other stuff, since it's got hooks into the
> GPIO and camera code too. But, like everything else I've posted so
> far, it's not the work that I personally set out to do. I can push
> that work on others :)
>
> That said, the suspend/resume support in this patch set makes suspend
> work on one chipset, and probably comes pretty close on the others
> without breaking anything there. I don't see the harm in merging it;
> it makes the code better than it is now. I would rather not have to
> separate it out from the rest. But I'll not fight over this one; if
> there's real opposition then we can force OLPC to continue to carry it
> out of tree.
At least I'd like some more time (multiple weeks) to have a look at this
issue & patches and try to come up with improvements that make it likely
work on other IGPs and eventually not needing any BIOS/OFW. I agree its
already an improvement but certainly one of the kind I like less as VIA
could come up with such "improvements" too. IMHO it's a bad thing to
push one chipset first if the target is to support all equally well (as
long as the hardware permits).
So I'd be glad if you could go on with the patches that are less painful
and get them ready for the merge window (as I really don't have a clue
how long it will take me to get the suspend/resume stuff to a state I
consider acceptable). Perhaps we should make suspend and resume a
configure option and mark it as experimental?
Thanks,
Florian Tobias Schandinat
Hi Jon + Florian,
On Fri, Apr 09, 2010 at 01:46:10PM -0600, Jonathan Corbet wrote:
> On Thu, 08 Apr 2010 20:42:17 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
> > something I am wondering about is whether we can't simply do:
> > viaparinfo->memsize = pci_resource_len(pdev, 0);
> > I suppose that this is not possible meaning that the pci len can be
> > longer than the actual memory but I just wanted to use the moment to
> > make sure.
>
> That would make sense. But if somebody who is closer to the hardware than
> I am doesn't take that approach, I'm nervous about changing it. Harald?
I believe it is not safe to simply use the pci_resource_len(pdev, 0).
At least in some of the hardware I've been working with in the past, the
PCI resource length was always fixed, independent of how large the BIOS
configured the shared video memory.
So please don't use that method.
Regards,
Harald
--
- Harald Welte <[email protected]> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
Hi Florian,
On Fri, Apr 09, 2010 at 11:40:55PM +0200, Florian Tobias Schandinat wrote:
> >The i2c transactions involved in detecting LVDS (9 seconds) and TMDS
> >(16 seconds) add an extra 25 seconds to viafb load time on the XO-1.5.
>
> I don't like the idea of OLPC specific code. Isn't there any way to
> speed this up in general?
I'm quite sure it can be sped up at least a bit... however:
> There is not yet even an option for OLPC_XO_1_5 (in contrast to
> CONFIG_OLPC) in mainline. Is such a thing planned?
> I can't really see anything that would speak for accepting this
> patch now in current mainline, sorry.
I think there is little choice in this matter. It is a _very_ uncommon
hardware design decision to attach anything but the DDC to one of the
I2C ports of the graphics chip, and the assumption that there is only
DDC connected is made in VIA's BIOS (not used in OLPC), the linux
framebuffer driver, as well as VIA's own Xorg driver and I believe as
well in OpenChrome, too :(
OLPC has told me that the particular chip that is attached to I2C is
also very timing sensitive and not 100% I2C compliant, so I think it is
the safe choice to not try to do DDC on that port for the OLPC 1.5.
--
- Harald Welte <[email protected]> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
On Sat, 10 April 2010 Florian Tobias Schandinat <[email protected]> wrote:
> Jonathan Corbet schrieb:
> > On Sat, 10 Apr 2010 01:32:36 +0200
> > Florian Tobias Schandinat <[email protected]> wrote:
> >
> >> Please correct me if I am wrong but the remaining 6 patches concerning
> >> suspend&resume look like a real big FIXME. So at the end it is expected
> >> to work only on VX855 and needs something called OFW?
> >
> > OFW = OpenFirmware. You could say BIOS instead. It's pretty normal to
> > expect the BIOS to put things into a semi-rational state at resume time.
>
> So its more or less the same I always do for testing the module.
>
> > Well, if we want to keep s/r out of tree, we can do that. It will
> > complicate the merge of the other stuff, since it's got hooks into the
> > GPIO and camera code too. But, like everything else I've posted so
> > far, it's not the work that I personally set out to do. I can push
> > that work on others :)
> >
> > That said, the suspend/resume support in this patch set makes suspend
> > work on one chipset, and probably comes pretty close on the others
> > without breaking anything there. I don't see the harm in merging it;
> > it makes the code better than it is now. I would rather not have to
> > separate it out from the rest. But I'll not fight over this one; if
> > there's real opposition then we can force OLPC to continue to carry it
> > out of tree.
>
> At least I'd like some more time (multiple weeks) to have a look at this
> issue & patches and try to come up with improvements that make it likely
> work on other IGPs and eventually not needing any BIOS/OFW. I agree its
> already an improvement but certainly one of the kind I like less as VIA
> could come up with such "improvements" too. IMHO it's a bad thing to
> push one chipset first if the target is to support all equally well (as
> long as the hardware permits).
> So I'd be glad if you could go on with the patches that are less painful
> and get them ready for the merge window (as I really don't have a clue
> how long it will take me to get the suspend/resume stuff to a state I
> consider acceptable). Perhaps we should make suspend and resume a
> configure option and mark it as experimental?
In my case (Commell LE 365) the BIOS offers an option to restore graphics
to text mode on resume so the manual call of 'fbset -a $MODE' works
pretty well, only the acceleration has issues.
I don't remember if accel can get revived with vanilla 2.6.34-rc2 if it
was disabled before suspend.
When I get time to, I will give these patches a try. A central GIT tree
where all viafb patches get collected would definitely be nice (even with
multiple semi-throw-away "topic" branches).
Thanks,
Bruno
Bruno Pr?mont schrieb:
> On Sat, 10 April 2010 Florian Tobias Schandinat <[email protected]> wrote:
>> Jonathan Corbet schrieb:
>>> Well, if we want to keep s/r out of tree, we can do that. It will
>>> complicate the merge of the other stuff, since it's got hooks into the
>>> GPIO and camera code too. But, like everything else I've posted so
>>> far, it's not the work that I personally set out to do. I can push
>>> that work on others :)
Good news! After some hours of hacking, damaging a filesystem and nearly
smashing my SD card I've found a promising way to implement suspend and
resume in viafb based on the first patch of this series. I think this is
something that can be done for the next merge window. I'll start a clean
implementation after Jon sent an updated patch series (including the
initial s&r implementation)
>> At least I'd like some more time (multiple weeks) to have a look at this
>> issue & patches and try to come up with improvements that make it likely
>> work on other IGPs and eventually not needing any BIOS/OFW. I agree its
>> already an improvement but certainly one of the kind I like less as VIA
>> could come up with such "improvements" too. IMHO it's a bad thing to
>> push one chipset first if the target is to support all equally well (as
>> long as the hardware permits).
Goal basically reached:
- likely to work on all IGPs as nothing directly IGP dependent is there
- works without OFW/BIOS
However:
- suspending is very slow, looks like it takes double the time compared
to before s&r support was added to viafb (this is also with only the
original patch)
- output device setting is messed up (example: before suspend the output
was on DVI, after resume on CRT)
- does not restore some values in the mmio but reinitializes it instead.
Do you need any special values restored, Jon?
> In my case (Commell LE 365) the BIOS offers an option to restore graphics
> to text mode on resume so the manual call of 'fbset -a $MODE' works
> pretty well, only the acceleration has issues.
> I don't remember if accel can get revived with vanilla 2.6.34-rc2 if it
> was disabled before suspend.
Acceleration doesn't work in 2.6.34-rc2 or later after resume (limited
only to the cursor, the rest works for me). It will work after the
patches I am going to do are applied.
> When I get time to, I will give these patches a try. A central GIT tree
> where all viafb patches get collected would definitely be nice (even with
> multiple semi-throw-away "topic" branches).
Yes, I guess that would be a good idea at least now where 2 people are
actively working on viafb. I have also a few minor patches ready I'll
send in a few days (after having repaired my development environment).
Best regards,
Florian Tobias Schandinat
[Getting back to the older stuff...]
On Fri, 09 Apr 2010 22:34:16 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> >> Just a minor nit:
> >> Could we change the default so that if someone adds support for a new
> >> IGP (and misses this function) we default to either the newest or
> >> preferably to none? I've just seen too much poorly maintained code in
> >> this driver and defaulting to the oldest is hence a bad idea.
> >> Otherwise it's fine.
> >
> > That would require making an exhaustive list of older chipset types.
> > It could probably be inferred through inspection of the code, but I
> > worry about making assumptions in this area...
>
> Such list already exists. gfx_chip_name = pdi->driver_data in hw.c (and
> only there) so what is needed is the list viafb_pci_table in viafbdev.c
> (relatively at the end) of all chips:
I've spent a bit of time looking at this. What's really needed is a
better way of abstracting the chip types so that we can maybe get rid
of all those switch statements throughout the driver. For the purposes
of getting this work in, I'm not quite prepared to make that change,
though I could certainly consider doing it in the future.
In the absence of that, the only course of action which makes sense is
to simply fail the initialization if an unknown chip type shows up
there. That's easy, and I can do it. But, given that this was a
"minor nit," can we leave it as-is for now? There's a *lot* of things
to clean up in this driver, I'd like to make it better a step at a time
rather than trying to do the whole thing at once.
Thanks,
jon
On Fri, 09 Apr 2010 23:27:30 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> What will happen if someone with no VX855 will try to enter 1200x900
> mode? Probably the world won't be destroyed but I have a really ugly
> feeling that the driver is not well prepared for this situation.
OK, I'm certainly exposing my relative lack of understanding of
framebuffer issues (I'm here to add a camera driver, remember? :),
but...is this really something that older chipsets can't handle? Or is
it just that nobody has had that size of panel before?
> Haven't
> tested yet as I'm waiting for the official forward port but I think it
> might be better to reschedule this patch for later or add some PLL
> values that are supposed to work (using/executing the formulas in the
> VIA open source X driver)
If it's really not acceptable, then it will stay out, but I would
rather get it merged. I will leave it in the series for now; it can
come out later if need be.
Thanks,
jon
Jonathan Corbet schrieb:
> [Getting back to the older stuff...]
>
> On Fri, 09 Apr 2010 22:34:16 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
>>>> Just a minor nit:
>>>> Could we change the default so that if someone adds support for a new
>>>> IGP (and misses this function) we default to either the newest or
>>>> preferably to none? I've just seen too much poorly maintained code in
>>>> this driver and defaulting to the oldest is hence a bad idea.
>>>> Otherwise it's fine.
>
> In the absence of that, the only course of action which makes sense is
> to simply fail the initialization if an unknown chip type shows up
> there. That's easy, and I can do it. But, given that this was a
> "minor nit," can we leave it as-is for now?
Yes, if you feel too uncomfortable with changing it and agree that the
whole stuff should be made more maintainable later on I am okay with
letting this in as is.
> There's a *lot* of things to clean up in this driver, I'd like to make it
> better a step at a time rather than trying to do the whole thing at once.
This is indeed very true.
Thanks,
Florian Tobias Schandinat
Jonathan Corbet schrieb:
> On Fri, 09 Apr 2010 23:27:30 +0200
> Florian Tobias Schandinat <[email protected]> wrote:
>
>> What will happen if someone with no VX855 will try to enter 1200x900
>> mode? Probably the world won't be destroyed but I have a really ugly
>> feeling that the driver is not well prepared for this situation.
>
> OK, I'm certainly exposing my relative lack of understanding of
> framebuffer issues (I'm here to add a camera driver, remember? :),
Well I wasn't a graphics guy and just wanted a working display driver.
> but...is this really something that older chipsets can't handle? Or is
> it just that nobody has had that size of panel before?
It's not the chipsets that are incapable (AFAIK) it's just that the
patch is only adding the right PLL values for frequency generation for
VX855 and set the values for all other chipsets to 0. Yeah that means
this panel size wasn't supported before (by the driver). 0 is certainly
not what we want and I think about replacing this whole fixed table with
the formulas used in VIAs OpenSource X driver (although those are a bit
more complicated I expected when I looked at unichrome). However this
will certainly be a big change that needs a lot of thought and testing
so nothing that will happen very soon.
I am just worried what will happen if one tries to set this mode with an
older chipset. I'm pretty sure that it won't work and won't fail gracefully.
>> Haven't
>> tested yet as I'm waiting for the official forward port but I think it
>> might be better to reschedule this patch for later or add some PLL
>> values that are supposed to work (using/executing the formulas in the
>> VIA open source X driver)
>
> If it's really not acceptable, then it will stay out, but I would
> rather get it merged. I will leave it in the series for now; it can
> come out later if need be.
Yeah please leave it in the series for now. I'd just appreciate it very
much if one were to generate the missing values before this gets into
mainline....or replacing this hardcoded values.
Thanks,
Florian Tobias Schandinat
Hi!
On Sun, Apr 18, 2010 at 11:34:30AM -0600, Jonathan Corbet wrote:
> In the absence of that, the only course of action which makes sense is
> to simply fail the initialization if an unknown chip type shows up
> there. That's easy, and I can do it. But, given that this was a
> "minor nit," can we leave it as-is for now? There's a *lot* of things
> to clean up in this driver, I'd like to make it better a step at a time
> rather than trying to do the whole thing at once.
i agree with that step by step strategy.
I always felt conservative about making too big changes. After all, it is hard
to find people who actually own and use all those various models of graphics
chips, and even more so: who want to try + test recent mainline and check for
regressions :(
VIA's resources are extremely limited, and they can barely complete the work
required for new products... I know it's unfortunate, but VIA is a very small
company.
Regards,
Harald
--
- Harald Welte <[email protected]> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
[Trying to get caught up again ... ]
On Tue, 13 Apr 2010 05:03:33 +0200
Florian Tobias Schandinat <[email protected]> wrote:
> Good news! After some hours of hacking, damaging a filesystem and nearly
> smashing my SD card I've found a promising way to implement suspend and
> resume in viafb based on the first patch of this series. I think this is
> something that can be done for the next merge window.
This all sounds good.
> I'll start a clean
> implementation after Jon sent an updated patch series (including the
> initial s&r implementation)
I'd missed that on my previous reading. Do you want me to post a new
S/R version over the latest patch set?
> However:
> - suspending is very slow, looks like it takes double the time compared
> to before s&r support was added to viafb (this is also with only the
> original patch)
You mean, double the time without actually dealing with the frame
buffer? I'm a little confused there.
> - does not restore some values in the mmio but reinitializes it instead.
> Do you need any special values restored, Jon?
Code using MMIO (the camera in particular) definitely needs stuff
done; the OLPC code can currently suspend and resume with the camera
streaming and it all works.
What I'd done was to move core S/R into via-core.c (which you've not
yet seen, but which I hope to get posted tomorrow) and give subdevices
a hook so they can be called for S/R events. That lets each subdev
ensure that its own needs are met.
Now, I'd been expecting to strip that out on the assumption that the
current S/R code wasn't going to make it upstream. I could change
plans, though, if that's helpful.
> > When I get time to, I will give these patches a try. A central GIT tree
> > where all viafb patches get collected would definitely be nice (even with
> > multiple semi-throw-away "topic" branches).
>
> Yes, I guess that would be a good idea at least now where 2 people are
> actively working on viafb. I have also a few minor patches ready I'll
> send in a few days (after having repaired my development environment).
As a starting point, I've set up:
git://git.lwn.net/linux-2.6.git viafb-next
That's for patches aimed at linux-next. It contains the last series I
sent out, plus one embarrassing compilation fix. Once I have other
stuff ready for review (soon!), I'll stick it into a different branch.
Incidentally, the "olpc-2.6.31-cam" branch there has the full set of
code as used by OLPC now. That branch reflects a lot history of
figuring out how this hardware actually works (sometimes it looks
vaguely like what's in the datasheet, but I think that's coincidental);
some real cleanup needs to happen on the way to the mainline.
Thanks,
jon