[PATCH 0/5] staging: sm7xxfb: few changes

[PATCH 0/5] staging: sm7xxfb: few changes

[Sudip Mukherjee]

Few more changes in sm7xxfb, and hopefully the last changes before I
send the patch to move it to drivers/video/fbdev.

Has been tested with:
1) fbtest available at https://git.kernel.org/cgit/linux/kernel/git/geert/fbtest.git/
2) read and write tested on Little-Endian system


regards
sudip

Sudip Mukherjee (5):
  staging: sm7xxfb: remove unused macros
  staging: sm7xxfb: fix error handling
  staging: sm7xxfb: use kernel commandline
  staging: sm7xxfb: define new macros
  staging: sm7xxfb: usr fb_read and fb_write

 drivers/staging/sm7xxfb/sm7xx.h   | 39 +++++++++++-----------
 drivers/staging/sm7xxfb/sm7xxfb.c | 69 ++++++++++-----------------------------
 2 files changed, 37 insertions(+), 71 deletions(-)

-- 
1.8.1.2

[PATCH 1/5] staging: sm7xxfb: remove unused macros

[Sudip Mukherjee]

These macros were only defined but not used anywhere.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm7xxfb/sm7xx.h | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/staging/sm7xxfb/sm7xx.h
index 4bed094..31a21bd 100644
--- a/drivers/staging/sm7xxfb/sm7xx.h
+++ b/drivers/staging/sm7xxfb/sm7xx.h
@@ -13,8 +13,6 @@
  *  more details.
  */
 
-#define NR_PALETTE        256
-
 #define FB_ACCEL_SMI_LYNX 88
 
 #define SCREEN_X_RES      1024
@@ -31,12 +29,8 @@
 
 extern void __iomem *smtc_regbaseaddress;
 #define smtc_mmiowb(dat, reg)	writeb(dat, smtc_regbaseaddress + reg)
-#define smtc_mmioww(dat, reg)	writew(dat, smtc_regbaseaddress + reg)
-#define smtc_mmiowl(dat, reg)	writel(dat, smtc_regbaseaddress + reg)
 
 #define smtc_mmiorb(reg)	readb(smtc_regbaseaddress + reg)
-#define smtc_mmiorw(reg)	readw(smtc_regbaseaddress + reg)
-#define smtc_mmiorl(reg)	readl(smtc_regbaseaddress + reg)
 
 #define SIZE_SR00_SR04      (0x04 - 0x00 + 1)
 #define SIZE_SR10_SR24      (0x24 - 0x10 + 1)
@@ -48,8 +42,6 @@ extern void __iomem *smtc_regbaseaddress;
 #define SIZE_CR00_CR18      (0x18 - 0x00 + 1)
 #define SIZE_CR30_CR4D      (0x4D - 0x30 + 1)
 #define SIZE_CR90_CRA7      (0xA7 - 0x90 + 1)
-#define SIZE_VPR		(0x6C + 1)
-#define SIZE_DPR		(0x44 + 1)
 
 static inline void smtc_crtcw(int reg, int val)
 {
@@ -57,24 +49,12 @@ static inline void smtc_crtcw(int reg, int val)
 	smtc_mmiowb(val, 0x3d5);
 }
 
-static inline unsigned int smtc_crtcr(int reg)
-{
-	smtc_mmiowb(reg, 0x3d4);
-	return smtc_mmiorb(0x3d5);
-}
-
 static inline void smtc_grphw(int reg, int val)
 {
 	smtc_mmiowb(reg, 0x3ce);
 	smtc_mmiowb(val, 0x3cf);
 }
 
-static inline unsigned int smtc_grphr(int reg)
-{
-	smtc_mmiowb(reg, 0x3ce);
-	return smtc_mmiorb(0x3cf);
-}
-
 static inline void smtc_attrw(int reg, int val)
 {
 	smtc_mmiorb(0x3da);
-- 
1.8.1.2

[PATCH 2/5] staging: sm7xxfb: fix error handling

[Sudip Mukherjee]

We were checking smtc_regbaseaddress and that too at a place where it
can never be NULL. Real check should be on sfb->lfb immediately after
we do ioremap.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 2ff4fe7..8fb62af 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -1456,6 +1456,14 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 #else
 		sfb->lfb = ioremap(mmio_base, 0x00800000);
 #endif
+		if (!sfb->lfb) {
+			dev_err(&pdev->dev,
+				"%s: unable to map memory mapped IO!\n",
+				sfb->fb->fix.id);
+			err = -ENOMEM;
+			goto failed_fb;
+		}
+
 		sfb->mmio = (smtc_regbaseaddress  		    sfb->lfb + 0x00700000);
 		sfb->dp_regs = sfb->lfb + 0x00408000;
@@ -1466,13 +1474,6 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 			dev_info(&pdev->dev, "sfb->lfb=%p\n", sfb->lfb);
 		}
 #endif
-		if (!smtc_regbaseaddress) {
-			dev_err(&pdev->dev,
-				"%s: unable to map memory mapped IO!\n",
-				sfb->fb->fix.id);
-			err = -ENOMEM;
-			goto failed_fb;
-		}
 
 		/* set MCLK = 14.31818 * (0x16 / 0x2) */
 		smtc_seqw(0x6a, 0x16);
-- 
1.8.1.2

[PATCH 3/5] staging: sm7xxfb: use kernel commandline

[Sudip Mukherjee]

We were only using the kernel commandline to set the mode if this driver
is builtin, but when it is built as a module we were not having any way
to set the mode. Start using commandline even if it is built as a
module.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 8fb62af..4dc9d5f 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -1660,14 +1660,12 @@ static struct pci_driver smtcfb_driver = {
 
 static int __init sm712fb_init(void)
 {
-#ifndef MODULE
 	char *option = NULL;
 
 	if (fb_get_options("sm712fb", &option))
 		return -ENODEV;
 	if (option && *option)
 		mode_option = option;
-#endif
 	sm7xx_vga_setup(mode_option);
 
 	return pci_register_driver(&smtcfb_driver);
-- 
1.8.1.2

[PATCH 4/5] staging: sm7xxfb: define new macros

[Sudip Mukherjee]

Define and use some new macros to work with different situations
based on little-endian and big-endian.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm7xxfb/sm7xx.h   | 19 ++++++++++++++++
 drivers/staging/sm7xxfb/sm7xxfb.c | 48 ++++++++-------------------------------
 2 files changed, 29 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/staging/sm7xxfb/sm7xx.h
index 31a21bd..6905177 100644
--- a/drivers/staging/sm7xxfb/sm7xx.h
+++ b/drivers/staging/sm7xxfb/sm7xx.h
@@ -95,3 +95,22 @@ struct modeinit {
 	unsigned char init_cr30_cr4d[SIZE_CR30_CR4D];
 	unsigned char init_cr90_cra7[SIZE_CR90_CRA7];
 };
+
+#ifdef __BIG_ENDIAN
+#define pal_rgb(r, g, b, val)	(((r & 0xf800) >> 8) | \
+				((g & 0xe000) >> 13) | \
+				((g & 0x1c00) << 3) | \
+				((b & 0xf800) >> 3))
+#define big_addr		0x800000
+#define mmio_addr		0x00800000
+#define seqw17			smtc_seqw(0x17, 0x30)
+#define big_pixel_depth(p, d)	{if (p = 24) {p = 32; d = 32; } }
+#define big_swap(p)		((p & 0xff00ff00 >> 8) | (p & 0x00ff00ff << 8))
+#else
+#define pal_rgb(r, g, b, val)	val
+#define big_addr		0
+#define mmio_addr		0x00c00000
+#define seqw17
+#define big_pixel_depth(p, d)
+#define big_swap(p)		p
+#endif
diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 4dc9d5f..252f110a 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -923,25 +923,14 @@ static int smtc_setcolreg(unsigned regno, unsigned red, unsigned green,
 			val = chan_to_field(red, &sfb->fb->var.red);
 			val |= chan_to_field(green, &sfb->fb->var.green);
 			val |= chan_to_field(blue, &sfb->fb->var.blue);
-#ifdef __BIG_ENDIAN
-			pal[regno] = ((red & 0xf800) >> 8) |
-				     ((green & 0xe000) >> 13) |
-				     ((green & 0x1c00) << 3) |
-				     ((blue & 0xf800) >> 3);
-#else
-			pal[regno] = val;
-#endif
+			pal[regno] = pal_rgb(red, green, blue, val);
 		} else {
 			u32 *pal = sfb->fb->pseudo_palette;
 
 			val = chan_to_field(red, &sfb->fb->var.red);
 			val |= chan_to_field(green, &sfb->fb->var.green);
 			val |= chan_to_field(blue, &sfb->fb->var.blue);
-#ifdef __BIG_ENDIAN
-			val = (val & 0xff00ff00 >> 8) |
-			      (val & 0x00ff00ff << 8);
-#endif
-			pal[regno] = val;
+			pal[regno] = big_swap(val);
 		}
 		break;
 
@@ -1002,8 +991,7 @@ static ssize_t smtcfb_read(struct fb_info *info, char __user *buf,
 		dst = buffer;
 		for (i = c >> 2; i--;) {
 			*dst = fb_readl(src++);
-			*dst = (*dst & 0xff00ff00 >> 8) |
-			       (*dst & 0x00ff00ff << 8);
+			*dst = big_swap(*dst);
 			dst++;
 		}
 		if (c & 3) {
@@ -1091,8 +1079,7 @@ static ssize_t smtcfb_write(struct fb_info *info, const char __user *buf,
 		}
 
 		for (i = c >> 2; i--;) {
-			fb_writel((*src & 0xff00ff00 >> 8) |
-				  (*src & 0x00ff00ff << 8), dst++);
+			fb_writel(big_swap(*src), dst++);
 			src++;
 		}
 		if (c & 3) {
@@ -1341,10 +1328,8 @@ static int smtc_map_smem(struct smtcfb_info *sfb,
 {
 	sfb->fb->fix.smem_start = pci_resource_start(pdev, 0);
 
-#ifdef __BIG_ENDIAN
 	if (sfb->fb->var.bits_per_pixel = 32)
-		sfb->fb->fix.smem_start += 0x800000;
-#endif
+		sfb->fb->fix.smem_start += big_addr;
 
 	sfb->fb->fix.smem_len = smem_len;
 
@@ -1437,10 +1422,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		sfb->fb->var.bits_per_pixel = SCREEN_BPP;
 	}
 
-#ifdef __BIG_ENDIAN
-	if (sfb->fb->var.bits_per_pixel = 24)
-		sfb->fb->var.bits_per_pixel = (smtc_scr_info.lfb_depth = 32);
-#endif
+	big_pixel_depth(sfb->fb->var.bits_per_pixel, smtc_scr_info.lfb_depth);
 	/* Map address and memory detection */
 	mmio_base = pci_resource_start(pdev, 0);
 	pci_read_config_byte(pdev, PCI_REVISION_ID, &sfb->chip_rev_id);
@@ -1451,11 +1433,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		sfb->fb->fix.mmio_start = mmio_base + 0x00400000;
 		sfb->fb->fix.mmio_len = 0x00400000;
 		smem_size = SM712_VIDEOMEMORYSIZE;
-#ifdef __BIG_ENDIAN
-		sfb->lfb = ioremap(mmio_base, 0x00c00000);
-#else
-		sfb->lfb = ioremap(mmio_base, 0x00800000);
-#endif
+		sfb->lfb = ioremap(mmio_base, mmio_addr);
 		if (!sfb->lfb) {
 			dev_err(&pdev->dev,
 				"%s: unable to map memory mapped IO!\n",
@@ -1468,12 +1446,10 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		    sfb->lfb + 0x00700000);
 		sfb->dp_regs = sfb->lfb + 0x00408000;
 		sfb->vp_regs = sfb->lfb + 0x0040c000;
-#ifdef __BIG_ENDIAN
 		if (sfb->fb->var.bits_per_pixel = 32) {
-			sfb->lfb += 0x800000;
+			sfb->lfb += big_addr;
 			dev_info(&pdev->dev, "sfb->lfb=%p\n", sfb->lfb);
 		}
-#endif
 
 		/* set MCLK = 14.31818 * (0x16 / 0x2) */
 		smtc_seqw(0x6a, 0x16);
@@ -1482,10 +1458,8 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		/* enable PCI burst */
 		smtc_seqw(0x17, 0x20);
 		/* enable word swap */
-#ifdef __BIG_ENDIAN
 		if (sfb->fb->var.bits_per_pixel = 32)
-			smtc_seqw(0x17, 0x30);
-#endif
+			seqw17;
 		break;
 	case 0x720:
 		sfb->fb->fix.mmio_start = mmio_base;
@@ -1617,10 +1591,8 @@ static int smtcfb_pci_resume(struct device *device)
 		smtc_seqw(0x62, 0x3e);
 		/* enable PCI burst */
 		smtc_seqw(0x17, 0x20);
-#ifdef __BIG_ENDIAN
 		if (sfb->fb->var.bits_per_pixel = 32)
-			smtc_seqw(0x17, 0x30);
-#endif
+			seqw17;
 		break;
 	case 0x720:
 		smtc_seqw(0x62, 0xff);
-- 
1.8.1.2

[PATCH 5/5] staging: sm7xxfb: usr fb_read and fb_write

[Sudip Mukherjee]

Now since the Big-Endian and Little-Endian based calculations are moved
into a macro we can make fb_read() and fb_write() common for both
Little-Endian and Big-Endian.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 252f110a..b8a1e86 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -946,7 +946,6 @@ static int smtc_setcolreg(unsigned regno, unsigned red, unsigned green,
 	return 0;
 }
 
-#ifdef __BIG_ENDIAN
 static ssize_t smtcfb_read(struct fb_info *info, char __user *buf,
 			   size_t count, loff_t *ppos)
 {
@@ -1107,7 +1106,6 @@ static ssize_t smtcfb_write(struct fb_info *info, const char __user *buf,
 
 	return (cnt) ? cnt : err;
 }
-#endif	/* ! __BIG_ENDIAN */
 
 static void sm7xx_set_timing(struct smtcfb_info *sfb)
 {
@@ -1303,10 +1301,8 @@ static struct fb_ops smtcfb_ops = {
 	.fb_fillrect  = cfb_fillrect,
 	.fb_imageblit = cfb_imageblit,
 	.fb_copyarea  = cfb_copyarea,
-#ifdef __BIG_ENDIAN
 	.fb_read      = smtcfb_read,
 	.fb_write     = smtcfb_write,
-#endif
 };
 
 /*
-- 
1.8.1.2

Re: [PATCH 4/5] staging: sm7xxfb: define new macros

[Greg Kroah-Hartman]

On Tue, Jul 07, 2015 at 01:44:36PM +0530, Sudip Mukherjee wrote:
> Define and use some new macros to work with different situations
> based on little-endian and big-endian.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>  drivers/staging/sm7xxfb/sm7xx.h   | 19 ++++++++++++++++
>  drivers/staging/sm7xxfb/sm7xxfb.c | 48 ++++++++-------------------------------
>  2 files changed, 29 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/staging/sm7xxfb/sm7xx.h
> index 31a21bd..6905177 100644
> --- a/drivers/staging/sm7xxfb/sm7xx.h
> +++ b/drivers/staging/sm7xxfb/sm7xx.h
> @@ -95,3 +95,22 @@ struct modeinit {
>  	unsigned char init_cr30_cr4d[SIZE_CR30_CR4D];
>  	unsigned char init_cr90_cra7[SIZE_CR90_CRA7];
>  };
> +
> +#ifdef __BIG_ENDIAN
> +#define pal_rgb(r, g, b, val)	(((r & 0xf800) >> 8) | \
> +				((g & 0xe000) >> 13) | \
> +				((g & 0x1c00) << 3) | \
> +				((b & 0xf800) >> 3))
> +#define big_addr		0x800000
> +#define mmio_addr		0x00800000
> +#define seqw17			smtc_seqw(0x17, 0x30)
> +#define big_pixel_depth(p, d)	{if (p = 24) {p = 32; d = 32; } }
> +#define big_swap(p)		((p & 0xff00ff00 >> 8) | (p & 0x00ff00ff << 8))
> +#else
> +#define pal_rgb(r, g, b, val)	val
> +#define big_addr		0
> +#define mmio_addr		0x00c00000
> +#define seqw17

Odd, empty macros are not good, because:

> -#ifdef __BIG_ENDIAN
>  		if (sfb->fb->var.bits_per_pixel = 32)
> -			smtc_seqw(0x17, 0x30);
> -#endif
> +			seqw17;

That just looks wrong :(

Re: [PATCH 3/5] staging: sm7xxfb: use kernel commandline

[Greg Kroah-Hartman]

On Tue, Jul 07, 2015 at 01:44:35PM +0530, Sudip Mukherjee wrote:
> We were only using the kernel commandline to set the mode if this driver
> is builtin, but when it is built as a module we were not having any way
> to set the mode. Start using commandline even if it is built as a
> module.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

I applied the first 3 patches here, but not 4 and 5.

Re: [PATCH 4/5] staging: sm7xxfb: define new macros

[Sudip Mukherjee]

On Tue, Jul 14, 2015 at 08:05:08PM -0700, Greg Kroah-Hartman wrote:
> On Tue, Jul 07, 2015 at 01:44:36PM +0530, Sudip Mukherjee wrote:
> > +#define mmio_addr		0x00c00000
> > +#define seqw17
> 
> Odd, empty macros are not good, because:
> 
> > -#ifdef __BIG_ENDIAN
> >  		if (sfb->fb->var.bits_per_pixel = 32)
> > -			smtc_seqw(0x17, 0x30);
> > -#endif
> > +			seqw17;
> 
> That just looks wrong :(
I think it will look better in c file if I make it as
#define seqw17()
I will respin the patches and send.

regards
sudip