Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v2 03/12] staging: sm750: move '{' to previous line
From: Michel von Czettritz @ 2015-03-26 22:25 UTC (permalink / raw)
  To: linux-fbdev

Fix checkpatch.pl error:
"ERROR: that open brace { should be on the previous line"

Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 50 +++++++++++++----------------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index e98eefc..bc2986d 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -154,8 +154,7 @@ static int lynxfb_ops_cursor(struct fb_info* info, struct fb_cursor* fbcursor)
 	}
 
 
-	if(fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE))
-	{
+	if(fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
 		cursor->setData(cursor,
 				fbcursor->rop,
 				fbcursor->image.data,
@@ -255,13 +254,10 @@ static void lynxfb_ops_imageblit(struct fb_info*info, const struct fb_image* ima
 
 	if(image->depth = 1){
 		if(info->fix.visual = FB_VISUAL_TRUECOLOR ||
-		   info->fix.visual = FB_VISUAL_DIRECTCOLOR)
-		{
+				info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
 			fgcol = ((u32*)info->pseudo_palette)[image->fg_color];
 			bgcol = ((u32*)info->pseudo_palette)[image->bg_color];
-		}
-		else
-		{
+		} else {
 			fgcol = image->fg_color;
 			bgcol = image->bg_color;
 		}
@@ -630,8 +626,7 @@ static int lynxfb_ops_setcolreg(unsigned regno, unsigned red,
 	if(info->var.grayscale)
 		red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
 
-	if(var->bits_per_pixel = 8 && info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
-	{
+	if(var->bits_per_pixel = 8 && info->fix.visual = FB_VISUAL_PSEUDOCOLOR) {
 		red >>= 8;
 		green >>= 8;
 		blue >>= 8;
@@ -640,13 +635,11 @@ static int lynxfb_ops_setcolreg(unsigned regno, unsigned red,
 	}
 
 
-	if(info->fix.visual = FB_VISUAL_TRUECOLOR && regno < 256 )
-	{
+	if(info->fix.visual = FB_VISUAL_TRUECOLOR && regno < 256 ) {
 		u32 val;
 		if(var->bits_per_pixel = 16 ||
 		   var->bits_per_pixel = 32 ||
-		   var->bits_per_pixel = 24)
-		{
+		   var->bits_per_pixel = 24) {
 			val = chan_to_field(red, &var->red);
 			val |= chan_to_field(green, &var->green);
 			val |= chan_to_field(blue, &var->blue);
@@ -709,8 +702,7 @@ static int sm750fb_set_drv(struct lynxfb_par * par)
 	output->clear = hw_sm750_output_clear;
 	/* chip specific phase */
 	share->accel.de_wait = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_deWait: hw_sm750_deWait;
-	switch (spec_share->state.dataflow)
-	{
+	switch (spec_share->state.dataflow) {
 	case sm750_simul_pri:
 		output->paths = sm750_pnc;
 		crtc->channel = sm750_primary;
@@ -794,8 +786,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info, int index)
 	};
 
 
-	static const char * fixId[2]-	{
+	static const char * fixId[2] = {
 		"sm750_fb1", "sm750_fb2",
 	};
 
@@ -935,8 +926,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info, int index)
 	pr_info("fix->mmio_start = %lx\n", fix->mmio_start);
 	fix->mmio_len = share->vidreg_size;
 	pr_info("fix->mmio_len = %x\n", fix->mmio_len);
-	switch(var->bits_per_pixel)
-	{
+	switch(var->bits_per_pixel) {
 	case 8:
 		fix->visual = FB_VISUAL_PSEUDOCOLOR;
 		break;
@@ -1028,8 +1018,7 @@ static void sm750fb_setup(struct lynx_share * share, char * src)
 			g_hwcursor &= ~0x2;
 		else if(!strncmp(opt, "nohwc", strlen("nohwc")))
 			g_hwcursor = 0;
-		else
-		{
+		else {
 			if(!g_fbmode[0]){
 				g_fbmode[0] = opt;
 				pr_info("find fbmode0 : %s\n", g_fbmode[0]);
@@ -1042,8 +1031,7 @@ static void sm750fb_setup(struct lynx_share * share, char * src)
 		}
 	}
 #ifdef CAP_EXPANSION
-	if(getExpRes(exp_res, &spec_share->state.xLCD, &spec_share->state.yLCD))
-	{
+	if(getExpRes(exp_res, &spec_share->state.xLCD, &spec_share->state.yLCD)) {
 		/* seems exp_res is not valid*/
 		spec_share->state.xLCD = spec_share->state.yLCD = 0;
 	}
@@ -1051,8 +1039,7 @@ static void sm750fb_setup(struct lynx_share * share, char * src)
 
 NO_PARAM:
 	if(share->revid != SM750LE_REVISION_ID){
-		if(share->dual)
-		{
+		if(share->dual) {
 			if(swap)
 				spec_share->state.dataflow = sm750_dual_swap;
 			else
@@ -1168,16 +1155,13 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	fbidx = 0;
 ALLOC_FB:
 	info[fbidx] = framebuffer_alloc(sizeof(struct lynxfb_par), &pdev->dev);
-	if(!info[fbidx])
-	{
+	if(!info[fbidx]) {
 		pr_err("Could not allocate framebuffer #%d.\n", fbidx);
 		if(fbidx = 0)
 			goto err_info0_alloc;
 		else
 			goto err_info1_alloc;
-	}
-	else
-	{
+	} else {
 		struct lynxfb_par * par;
 		int errno;
 		pr_info("framebuffer #%d alloc okay\n", fbidx);
@@ -1296,8 +1280,7 @@ static int __init lynxfb_setup(char * options)
 		strsep() updates @options to pointer after the first found token
 		it also returns the pointer ahead the token.
 		*/
-	while((opt = strsep(&options, ":"))!=NULL)
-	{
+	while((opt = strsep(&options, ":"))!=NULL) {
 		/* options that mean for any lynx chips are configured here */
 		if(!strncmp(opt, "noaccel", strlen("noaccel")))
 			g_noaccel = 1;
@@ -1307,8 +1290,7 @@ static int __init lynxfb_setup(char * options)
 #endif
 		else if(!strncmp(opt, "dual", strlen("dual")))
 			g_dualview = 1;
-		else
-		{
+		else {
 			strcat(tmp, opt);
 			tmp += strlen(opt);
 			if(options != NULL)
-- 
2.3.3


^ permalink raw reply related

* [PATCH v2 02/12] staging: sm750: add missing spaces after ','
From: Michel von Czettritz @ 2015-03-26 22:24 UTC (permalink / raw)
  To: linux-fbdev

Fixes checkpatch.pl error:
"ERROR: space required after that ',' (ctx:VxV)"

Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
 drivers/staging/sm750fb/sm750.c | 264 ++++++++++++++++++++--------------------
 1 file changed, 132 insertions(+), 132 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 87b50fc..e98eefc 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -50,26 +50,26 @@ static int g_noaccel;
 #ifdef CONFIG_MTRR
 static int g_nomtrr;
 #endif
-static const char * g_fbmode[] = {NULL,NULL};
+static const char * g_fbmode[] = {NULL, NULL};
 static const char * g_def_fbmode = "800x600-16@60";
 static char * g_settings = NULL;
 static int g_dualview;
 static char * g_option = NULL;
 
 static const struct fb_videomode lynx750_ext[] = {
-	/*	1024x600-60 VESA	[1.71:1]	*/
+	/*  	1024x600-60 VESA 	[1.71:1]	*/
 	{NULL,  60, 1024, 600, 20423, 144,  40, 18, 1, 104, 3,
 	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*	1024x600-70 VESA */
+	/* 	1024x600-70 VESA */
 	{NULL,  70, 1024, 600, 17211, 152,  48, 21, 1, 104, 3,
 	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*	1024x600-75 VESA */
+	/*  	1024x600-75 VESA */
 	{NULL,  75, 1024, 600, 15822, 160,  56, 23, 1, 104, 3,
 	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*	1024x600-85 VESA */
+	/*  	1024x600-85 VESA */
 	{NULL,  85, 1024, 600, 13730, 168,  56, 26, 1, 112, 3,
 	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
@@ -79,13 +79,13 @@ static const struct fb_videomode lynx750_ext[] = {
 
 	/*	1280x720		[1.78:1]	*/
 	{NULL, 60,  1280,  720,  13426, 162, 86, 22, 1,  136, 3,
-	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*	1280x768@60 */
-	{NULL,60,1280,768,12579,192,64,20,3,128,7,
+	/* 1280x768@60 */
+	{NULL, 60, 1280, 768, 12579, 192, 64, 20, 3, 128, 7,
 	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
 
-	{NULL,60,1360,768,11804,208,64,23,1,144,3,
+	{NULL, 60, 1360, 768, 11804, 208, 64, 23, 1, 144, 3,
 	 FB_SYNC_HOR_HIGH_ACT|FB_VMODE_NONINTERLACED},
 
 	/*	1360 x 768	[1.77083:1]	*/
@@ -96,9 +96,9 @@ static const struct fb_videomode lynx750_ext[] = {
 	{NULL, 60,  1368,  768,  11647, 216, 72, 23, 1,  144, 3,
 	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*	1440 x 900		[16:10]	*/
+	/* 	1440 x 900		[16:10]	*/
 	{NULL, 60, 1440, 900, 9392, 232, 80, 28, 1, 152, 3,
-	 FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
 	/*	1440x960		[15:10]	*/
 	{NULL, 60, 1440, 960, 8733, 240, 88, 30, 1, 152, 3,
@@ -106,14 +106,14 @@ static const struct fb_videomode lynx750_ext[] = {
 
 	/*	1920x1080	[16:9]	*/
 	{NULL, 60, 1920, 1080, 6734, 148, 88, 41, 1, 44, 3,
-	 FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 };
 
 
 
 
 /* no hardware cursor supported under version 2.6.10, kernel bug */
-static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
+static int lynxfb_ops_cursor(struct fb_info* info, struct fb_cursor* fbcursor)
 {
 	struct lynxfb_par * par;
 	struct lynxfb_crtc * crtc;
@@ -131,17 +131,17 @@ static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
 
 	cursor->disable(cursor);
 	if(fbcursor->set & FB_CUR_SETSIZE){
-		cursor->setSize(cursor,fbcursor->image.width,fbcursor->image.height);
+		cursor->setSize(cursor, fbcursor->image.width, fbcursor->image.height);
 	}
 
 	if(fbcursor->set & FB_CUR_SETPOS){
-		cursor->setPos(cursor,fbcursor->image.dx - info->var.xoffset,
+		cursor->setPos(cursor, fbcursor->image.dx - info->var.xoffset,
 			       fbcursor->image.dy - info->var.yoffset);
 	}
 
 	if(fbcursor->set & FB_CUR_SETCMAP){
 		/* get the 16bit color of kernel means */
-		u16 fg,bg;
+		u16 fg, bg;
 		fg = ((info->cmap.red[fbcursor->image.fg_color] & 0xf800))|
 		      ((info->cmap.green[fbcursor->image.fg_color] & 0xfc00) >> 5)|
 		      ((info->cmap.blue[fbcursor->image.fg_color] & 0xf800) >> 11);
@@ -150,7 +150,7 @@ static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
 		      ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5)|
 		      ((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
 
-		cursor->setColor(cursor,fg,bg);
+		cursor->setColor(cursor, fg, bg);
 	}
 
 
@@ -169,11 +169,11 @@ static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
 	return 0;
 }
 
-static void lynxfb_ops_fillrect(struct fb_info* info,const struct fb_fillrect* region)
+static void lynxfb_ops_fillrect(struct fb_info* info, const struct fb_fillrect* region)
 {
 	struct lynxfb_par * par;
 	struct lynx_share * share;
-	unsigned int base,pitch,Bpp,rop;
+	unsigned int base, pitch, Bpp, rop;
 	u32 color;
 
 	if(info->state != FBINFO_STATE_RUNNING){
@@ -200,19 +200,19 @@ static void lynxfb_ops_fillrect(struct fb_info* info,const struct fb_fillrect* r
 		spin_lock(&share->slock);
 
 	share->accel.de_fillrect(&share->accel,
-				 base,pitch,Bpp,
-				 region->dx,region->dy,
-				 region->width,region->height,
-				 color,rop);
+				 base, pitch, Bpp,
+				 region->dx, region->dy,
+				 region->width, region->height,
+				 color, rop);
 	if (share->dual)
 		spin_unlock(&share->slock);
 }
 
-static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea * region)
+static void lynxfb_ops_copyarea(struct fb_info * info, const struct fb_copyarea * region)
 {
 	struct lynxfb_par * par;
 	struct lynx_share * share;
-	unsigned int base,pitch,Bpp;
+	unsigned int base, pitch, Bpp;
 
 	par = info->par;
 	share = par->share;
@@ -231,17 +231,17 @@ static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea *
 		spin_lock(&share->slock);
 
 	share->accel.de_copyarea(&share->accel,
-				 base,pitch,region->sx,region->sy,
-				 base,pitch,Bpp,region->dx,region->dy,
-				 region->width,region->height,HW_ROP2_COPY);
+				 base, pitch, region->sx, region->sy,
+				 base, pitch, Bpp, region->dx, region->dy,
+				 region->width, region->height, HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
 }
 
-static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* image)
+static void lynxfb_ops_imageblit(struct fb_info*info, const struct fb_image* image)
 {
-	unsigned int base,pitch,Bpp;
-	unsigned int fgcol,bgcol;
+	unsigned int base, pitch, Bpp;
+	unsigned int fgcol, bgcol;
 	struct lynxfb_par * par;
 	struct lynx_share * share;
 
@@ -277,11 +277,11 @@ _do_work:
 		spin_lock(&share->slock);
 
 	share->accel.de_imageblit(&share->accel,
-				  image->data,image->width>>3,0,
-				  base,pitch,Bpp,
-				  image->dx,image->dy,
-				  image->width,image->height,
-				  fgcol,bgcol,HW_ROP2_COPY);
+				  image->data, image->width>>3, 0,
+				  base, pitch, Bpp,
+				  image->dx, image->dy,
+				  image->width, image->height,
+				  fgcol, bgcol, HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
 }
@@ -329,9 +329,9 @@ static int lynxfb_ops_set_par(struct fb_info * info)
 
 	/* fix structur is not so FIX ... */
 	line_length = var->xres_virtual * var->bits_per_pixel / 8;
-	line_length = PADDING(crtc->line_pad,line_length);
+	line_length = PADDING(crtc->line_pad, line_length);
 	fix->line_length = line_length;
-	pr_err("fix->line_length = %d\n",fix->line_length);
+	pr_err("fix->line_length = %d\n", fix->line_length);
 
 	/* var->red,green,blue,transp are need to be set by driver
 	 * and these data should be set before setcolreg routine
@@ -381,13 +381,13 @@ static int lynxfb_ops_set_par(struct fb_info * info)
 		pr_err("pixel bpp format not satisfied\n.");
 		return ret;
 	}
-	ret = crtc->proc_setMode(crtc,var,fix);
+	ret = crtc->proc_setMode(crtc, var, fix);
 	if(!ret)
-		ret = output->proc_setMode(output,var,fix);
+		ret = output->proc_setMode(output, var, fix);
 	return ret;
 }
 
-static inline unsigned int chan_to_field(unsigned int chan,struct fb_bitfield * bf)
+static inline unsigned int chan_to_field(unsigned int chan, struct fb_bitfield * bf)
 {
 	chan &= 0xffff;
 	chan >>= 16 - bf->length;
@@ -465,7 +465,7 @@ static int lynxfb_resume(struct pci_dev* pdev)
 	console_lock();
 
 	if((ret = pci_set_power_state(pdev, PCI_D0)) != 0){
-		pr_err("error:%d occured in pci_set_power_state\n",ret);
+		pr_err("error:%d occured in pci_set_power_state\n", ret);
 		return ret;
 	}
 
@@ -473,7 +473,7 @@ static int lynxfb_resume(struct pci_dev* pdev)
 	if(pdev->dev.power.power_state.event != PM_EVENT_FREEZE){
 		pci_restore_state(pdev);
 		if ((ret = pci_enable_device(pdev)) != 0){
-			pr_err("error:%d occured in pci_enable_device\n",ret);
+			pr_err("error:%d occured in pci_enable_device\n", ret);
 			return ret;
 		}
 		pci_set_master(pdev);
@@ -481,7 +481,7 @@ static int lynxfb_resume(struct pci_dev* pdev)
 	if(share->resume)
 		(*share->resume)(share);
 
-	hw_sm750_inithw(share,pdev);
+	hw_sm750_inithw(share, pdev);
 
 
 	info = share->fbinfo[0];
@@ -514,7 +514,7 @@ static int lynxfb_resume(struct pci_dev* pdev)
 }
 #endif
 
-static int lynxfb_ops_check_var(struct fb_var_screeninfo* var,struct fb_info* info)
+static int lynxfb_ops_check_var(struct fb_var_screeninfo* var, struct fb_info* info)
 {
 	struct lynxfb_par * par;
 	struct lynxfb_crtc * crtc;
@@ -543,7 +543,7 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo* var,struct fb_info* in
 	case 32:
 		break;
 	default:
-		pr_err("bpp %d not supported\n",var->bits_per_pixel);
+		pr_err("bpp %d not supported\n", var->bits_per_pixel);
 		ret = -EINVAL;
 		goto exit;
 	}
@@ -592,24 +592,24 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo* var,struct fb_info* in
 	request = var->xres_virtual * (var->bits_per_pixel >> 3);
 	/* defaulty crtc->channel go with par->index */
 
-	request = PADDING(crtc->line_pad,request);
+	request = PADDING(crtc->line_pad, request);
 	request = request * var->yres_virtual;
 	if(crtc->vidmem_size < request){
 		pr_err("not enough video memory for mode\n");
 		return -ENOMEM;
 	}
 
-	ret = output->proc_checkMode(output,var);
+	ret = output->proc_checkMode(output, var);
 	if(!ret)
-		ret = crtc->proc_checkMode(crtc,var);
+		ret = crtc->proc_checkMode(crtc, var);
 exit:
 	return ret;
 }
 
 
-static int lynxfb_ops_setcolreg(unsigned regno,unsigned red,
-				unsigned green,unsigned blue,
-				unsigned transp,struct fb_info * info)
+static int lynxfb_ops_setcolreg(unsigned regno, unsigned red,
+				unsigned green, unsigned blue,
+				unsigned transp, struct fb_info * info)
 {
 	struct lynxfb_par * par;
 	struct lynxfb_crtc * crtc;
@@ -623,7 +623,7 @@ static int lynxfb_ops_setcolreg(unsigned regno,unsigned red,
 
 	//pr_debug("regno=%d,red=%d,green=%d,blue=%d\n",regno,red,green,blue);
 	if(regno > 256){
-		pr_err("regno = %d\n",regno);
+		pr_err("regno = %d\n", regno);
 		return -EINVAL;
 	}
 
@@ -635,7 +635,7 @@ static int lynxfb_ops_setcolreg(unsigned regno,unsigned red,
 		red >>= 8;
 		green >>= 8;
 		blue >>= 8;
-		ret = crtc->proc_setColReg(crtc,regno,red,green,blue);
+		ret = crtc->proc_setColReg(crtc, regno, red, green, blue);
 		goto exit;
 	}
 
@@ -647,9 +647,9 @@ static int lynxfb_ops_setcolreg(unsigned regno,unsigned red,
 		   var->bits_per_pixel = 32 ||
 		   var->bits_per_pixel = 24)
 		{
-			val = chan_to_field(red,&var->red);
-			val |= chan_to_field(green,&var->green);
-			val |= chan_to_field(blue,&var->blue);
+			val = chan_to_field(red, &var->red);
+			val |= chan_to_field(green, &var->green);
+			val |= chan_to_field(blue, &var->blue);
 			par->pseudo_palette[regno] = val;
 			goto exit;
 		}
@@ -661,15 +661,15 @@ exit:
 	return ret;
 }
 
-static int lynxfb_ops_blank(int blank,struct fb_info* info)
+static int lynxfb_ops_blank(int blank, struct fb_info* info)
 {
 	struct lynxfb_par * par;
 	struct lynxfb_output * output;
 
-	pr_debug("blank = %d.\n",blank);
+	pr_debug("blank = %d.\n", blank);
 	par = info->par;
 	output = &par->output;
-	return output->proc_setBLANK(output,blank);
+	return output->proc_setBLANK(output, blank);
 }
 
 static int sm750fb_set_drv(struct lynxfb_par * par)
@@ -683,7 +683,7 @@ static int sm750fb_set_drv(struct lynxfb_par * par)
 	ret = 0;
 
 	share = par->share;
-	spec_share = container_of(share,struct sm750_share,share);
+	spec_share = container_of(share, struct sm750_share, share);
 	output = &par->output;
 	crtc = &par->crtc;
 
@@ -773,7 +773,7 @@ static struct fb_ops lynxfb_ops={
 };
 
 
-static int lynxfb_set_fbinfo(struct fb_info* info,int index)
+static int lynxfb_set_fbinfo(struct fb_info* info, int index)
 {
 	int i;
 	struct lynxfb_par * par;
@@ -784,9 +784,9 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 	struct fb_fix_screeninfo * fix;
 
 	const struct fb_videomode * pdb[] = {
-		lynx750_ext, NULL,vesa_modes,
+		lynx750_ext, NULL, vesa_modes,
 	};
-	int cdb[] = {ARRAY_SIZE(lynx750_ext),0,VESA_MODEDB_SIZE};
+	int cdb[] = {ARRAY_SIZE(lynx750_ext), 0, VESA_MODEDB_SIZE};
 	static const char * mdb_desc[] ={
 		"driver prepared modes",
 		"kernel prepared default modedb",
@@ -796,10 +796,10 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 
 	static const char * fixId[2] 	{
-		"sm750_fb1","sm750_fb2",
+		"sm750_fb1", "sm750_fb2",
 	};
 
-	int ret,line_length;
+	int ret, line_length;
 
 	ret = 0;
 	par = (struct lynxfb_par *)info->par;
@@ -821,7 +821,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 	crtc->cursor.offset = crtc->oScreen + crtc->vidmem_size - 1024;
 	crtc->cursor.mmio = share->pvReg + 0x800f0 + (int)crtc->channel * 0x140;
 
-	pr_info("crtc->cursor.mmio = %p\n",crtc->cursor.mmio);
+	pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
 	crtc->cursor.maxH = crtc->cursor.maxW = 64;
 	crtc->cursor.size = crtc->cursor.maxH*crtc->cursor.maxW*2/8;
 	crtc->cursor.disable = hw_cursor_disable;
@@ -859,8 +859,8 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 
 	for(i=0;i<3;i++){
 
-		ret = fb_find_mode(var,info,g_fbmode[index],
-				   pdb[i],cdb[i],NULL,8);
+		ret = fb_find_mode(var, info, g_fbmode[index],
+				   pdb[i], cdb[i], NULL, 8);
 
 		if(ret = 1){
 			pr_info("success! use specified mode:%s in %s\n",
@@ -878,7 +878,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 		}else if(ret = 4){
 			pr_warn("fall back to any valid mode\n");
 		}else{
-			pr_warn("ret = %d,fb_find_mode failed,with %s\n",ret,mdb_desc[i]);
+			pr_warn("ret = %d,fb_find_mode failed,with %s\n", ret, mdb_desc[i]);
 		}
 	}
 
@@ -892,8 +892,8 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 			xoffset=%d\n\
 			yoffset=%d\n\
 			bits_per_pixel=%d\n \
-			...\n",var->xres,var->yres,var->xres_virtual,var->yres_virtual,
-			var->xoffset,var->yoffset,var->bits_per_pixel);
+			...\n", var->xres, var->yres, var->xres_virtual, var->yres_virtual,
+			var->xoffset, var->yoffset, var->bits_per_pixel);
 
 	/* set par */
 	par->info = info;
@@ -904,7 +904,7 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 
 	info->pseudo_palette = &par->pseudo_palette[0];
 	info->screen_base = crtc->vScreen;
-	pr_debug("screen_base vaddr = %p\n",info->screen_base);
+	pr_debug("screen_base vaddr = %p\n", info->screen_base);
 	info->screen_size = line_length * var->yres_virtual;
 	info->flags = FBINFO_FLAG_DEFAULT|0;
 
@@ -916,11 +916,11 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 	fix->ywrapstep = crtc->ywrapstep;
 	fix->accel = FB_ACCEL_SMI;
 
-	strlcpy(fix->id,fixId[index],sizeof(fix->id));
+	strlcpy(fix->id, fixId[index], sizeof(fix->id));
 
 
 	fix->smem_start = crtc->oScreen + share->vidmem_start;
-	pr_info("fix->smem_start = %lx\n",fix->smem_start);
+	pr_info("fix->smem_start = %lx\n", fix->smem_start);
 	/* according to mmap experiment from user space application,
 	 * fix->mmio_len should not larger than virtual size
 	 * (xres_virtual x yres_virtual x ByPP)
@@ -928,13 +928,13 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 	 * data into the bound over virtual size
 	 * */
 	fix->smem_len = crtc->vidmem_size;
-	pr_info("fix->smem_len = %x\n",fix->smem_len);
+	pr_info("fix->smem_len = %x\n", fix->smem_len);
 	info->screen_size = fix->smem_len;
 	fix->line_length = line_length;
 	fix->mmio_start = share->vidreg_start;
-	pr_info("fix->mmio_start = %lx\n",fix->mmio_start);
+	pr_info("fix->mmio_start = %lx\n", fix->mmio_start);
 	fix->mmio_len = share->vidreg_size;
-	pr_info("fix->mmio_len = %x\n",fix->mmio_len);
+	pr_info("fix->mmio_len = %x\n", fix->mmio_len);
 	switch(var->bits_per_pixel)
 	{
 	case 8:
@@ -952,28 +952,28 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 	var->vmode = FB_VMODE_NONINTERLACED;
 
 	pr_debug("#1 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
-		 info->cmap.start,info->cmap.len,
-		 info->cmap.red,info->cmap.green,info->cmap.blue,
+		 info->cmap.start, info->cmap.len,
+		 info->cmap.red, info->cmap.green, info->cmap.blue,
 		 info->cmap.transp);
 
-	if((ret = fb_alloc_cmap(&info->cmap,256,0)) < 0){
+	if((ret = fb_alloc_cmap(&info->cmap, 256, 0)) < 0){
 		pr_err("Could not allcate memory for cmap.\n");
 		goto exit;
 	}
 
 	pr_debug("#2 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
-		 info->cmap.start,info->cmap.len,
-		 info->cmap.red,info->cmap.green,info->cmap.blue,
+		 info->cmap.start, info->cmap.len,
+		 info->cmap.red, info->cmap.green, info->cmap.blue,
 		 info->cmap.transp);
 
 exit:
-	lynxfb_ops_check_var(var,info);
+	lynxfb_ops_check_var(var, info);
 	//    lynxfb_ops_set_par(info);
 	return ret;
 }
 
 /*	chip specific g_option configuration routine */
-static void sm750fb_setup(struct lynx_share * share,char * src)
+static void sm750fb_setup(struct lynx_share * share, char * src)
 {
 	struct sm750_share * spec_share;
 	char * opt;
@@ -983,7 +983,7 @@ static void sm750fb_setup(struct lynx_share * share,char * src)
 	int swap;
 
 
-	spec_share = container_of(share,struct sm750_share,share);
+	spec_share = container_of(share, struct sm750_share, share);
 #ifdef CAP_EXPENSIION
 	exp_res = NULL;
 #endif
@@ -1004,45 +1004,45 @@ static void sm750fb_setup(struct lynx_share * share,char * src)
 		goto NO_PARAM;
 	}
 
-	while((opt = strsep(&src,":")) != NULL && *opt != 0){
-		pr_err("opt=%s\n",opt);
-		pr_err("src=%s\n",src);
+	while((opt = strsep(&src, ":")) != NULL && *opt != 0){
+		pr_err("opt=%s\n", opt);
+		pr_err("src=%s\n", src);
 
-		if(!strncmp(opt,"swap",strlen("swap")))
+		if(!strncmp(opt, "swap", strlen("swap")))
 			swap = 1;
-		else if(!strncmp(opt,"nocrt",strlen("nocrt")))
+		else if(!strncmp(opt, "nocrt", strlen("nocrt")))
 			spec_share->state.nocrt = 1;
-		else if(!strncmp(opt,"36bit",strlen("36bit")))
+		else if(!strncmp(opt, "36bit", strlen("36bit")))
 			spec_share->state.pnltype = sm750_doubleTFT;
-		else if(!strncmp(opt,"18bit",strlen("18bit")))
+		else if(!strncmp(opt, "18bit", strlen("18bit")))
 			spec_share->state.pnltype = sm750_dualTFT;
-		else if(!strncmp(opt,"24bit",strlen("24bit")))
+		else if(!strncmp(opt, "24bit", strlen("24bit")))
 			spec_share->state.pnltype = sm750_24TFT;
 #ifdef CAP_EXPANSION
-		else if(!strncmp(opt,"exp:",strlen("exp:")))
+		else if(!strncmp(opt, "exp:", strlen("exp:")))
 			exp_res = opt + strlen("exp:");
 #endif
-		else if(!strncmp(opt,"nohwc0",strlen("nohwc0")))
+		else if(!strncmp(opt, "nohwc0", strlen("nohwc0")))
 			g_hwcursor &= ~0x1;
-		else if(!strncmp(opt,"nohwc1",strlen("nohwc1")))
+		else if(!strncmp(opt, "nohwc1", strlen("nohwc1")))
 			g_hwcursor &= ~0x2;
-		else if(!strncmp(opt,"nohwc",strlen("nohwc")))
+		else if(!strncmp(opt, "nohwc", strlen("nohwc")))
 			g_hwcursor = 0;
 		else
 		{
 			if(!g_fbmode[0]){
 				g_fbmode[0] = opt;
-				pr_info("find fbmode0 : %s\n",g_fbmode[0]);
+				pr_info("find fbmode0 : %s\n", g_fbmode[0]);
 			}else if(!g_fbmode[1]){
 				g_fbmode[1] = opt;
-				pr_info("find fbmode1 : %s\n",g_fbmode[1]);
+				pr_info("find fbmode1 : %s\n", g_fbmode[1]);
 			}else{
 				pr_warn("How many view you wann set?\n");
 			}
 		}
 	}
 #ifdef CAP_EXPANSION
-	if(getExpRes(exp_res,&spec_share->state.xLCD,&spec_share->state.yLCD))
+	if(getExpRes(exp_res, &spec_share->state.xLCD, &spec_share->state.yLCD))
 	{
 		/* seems exp_res is not valid*/
 		spec_share->state.xLCD = spec_share->state.yLCD = 0;
@@ -1074,7 +1074,7 @@ NO_PARAM:
 static int lynxfb_pci_probe(struct pci_dev * pdev,
 			    const struct pci_device_id * ent)
 {
-	struct fb_info * info[] = {NULL,NULL};
+	struct fb_info * info[] = {NULL, NULL};
 	struct lynx_share * share = NULL;
 
 	struct sm750_share *spec_share = NULL;
@@ -1090,9 +1090,9 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 
 	/* though offset of share in sm750_share is 0,
 	 * we use this marcro as the same */
-	spec_offset = offsetof(struct sm750_share,share);
+	spec_offset = offsetof(struct sm750_share, share);
 
-	spec_share = kzalloc(sizeof(*spec_share),GFP_KERNEL);
+	spec_share = kzalloc(sizeof(*spec_share), GFP_KERNEL);
 	if(!spec_share){
 		pr_err("Could not allocate memory for share.\n");
 		goto err_share;
@@ -1104,7 +1104,7 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	share->devid = pdev->device;
 	share->revid = pdev->revision;
 
-	pr_info("share->revid = %02x\n",share->revid);
+	pr_info("share->revid = %02x\n", share->revid);
 	share->pdev = pdev;
 #ifdef CONFIG_MTRR
 	share->mtrr_off = g_nomtrr;
@@ -1130,10 +1130,10 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	}
 
 	/* call chip specific setup routine  */
-	sm750fb_setup(share,g_settings);
+	sm750fb_setup(share, g_settings);
 
 	/* call chip specific mmap routine */
-	if(hw_sm750_map(share,pdev)){
+	if(hw_sm750_map(share, pdev)){
 		pr_err("Memory map failed\n");
 		goto err_map;
 	}
@@ -1143,7 +1143,7 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 		pr_info("enable mtrr\n");
 		share->mtrr.vram = mtrr_add(share->vidmem_start,
 					    share->vidmem_size,
-					    MTRR_TYPE_WRCOMB,1);
+					    MTRR_TYPE_WRCOMB, 1);
 
 		if(share->mtrr.vram < 0){
 			/* don't block driver with the failure of MTRR */
@@ -1157,20 +1157,20 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 
 	memset_io(share->pvMem, 0, share->vidmem_size);
 
-	pr_info("sm%3x mmio address = %p\n",share->devid,share->pvReg);
+	pr_info("sm%3x mmio address = %p\n", share->devid, share->pvReg);
 
-	pci_set_drvdata(pdev,share);
+	pci_set_drvdata(pdev, share);
 
 	/* call chipInit routine */
-	hw_sm750_inithw(share,pdev);
+	hw_sm750_inithw(share, pdev);
 
 	/* allocate frame buffer info structor according to g_dualview */
 	fbidx = 0;
 ALLOC_FB:
-	info[fbidx] = framebuffer_alloc(sizeof(struct lynxfb_par),&pdev->dev);
+	info[fbidx] = framebuffer_alloc(sizeof(struct lynxfb_par), &pdev->dev);
 	if(!info[fbidx])
 	{
-		pr_err("Could not allocate framebuffer #%d.\n",fbidx);
+		pr_err("Could not allocate framebuffer #%d.\n", fbidx);
 		if(fbidx = 0)
 			goto err_info0_alloc;
 		else
@@ -1180,14 +1180,14 @@ ALLOC_FB:
 	{
 		struct lynxfb_par * par;
 		int errno;
-		pr_info("framebuffer #%d alloc okay\n",fbidx);
+		pr_info("framebuffer #%d alloc okay\n", fbidx);
 		share->fbinfo[fbidx] = info[fbidx];
 		par = info[fbidx]->par;
 		par->share = share;
 
 		/* set fb_info structure */
-		if(lynxfb_set_fbinfo(info[fbidx],fbidx)){
-			pr_err("Failed to initial fb_info #%d.\n",fbidx);
+		if(lynxfb_set_fbinfo(info[fbidx], fbidx)){
+			pr_err("Failed to initial fb_info #%d.\n", fbidx);
 			if(fbidx = 0)
 				goto err_info0_set;
 			else
@@ -1195,16 +1195,16 @@ ALLOC_FB:
 		}
 
 		/* register frame buffer*/
-		pr_info("Ready to register framebuffer #%d.\n",fbidx);
+		pr_info("Ready to register framebuffer #%d.\n", fbidx);
 		errno = register_framebuffer(info[fbidx]);
 		if (errno < 0) {
-			pr_err("Failed to register fb_info #%d. err %d\n",fbidx, errno);
+			pr_err("Failed to register fb_info #%d. err %d\n", fbidx, errno);
 			if(fbidx = 0)
 				goto err_register0;
 			else
 				goto err_register1;
 		}
-		pr_info("Accomplished register framebuffer #%d.\n",fbidx);
+		pr_info("Accomplished register framebuffer #%d.\n", fbidx);
 	}
 
 	/* no dual view by far */
@@ -1256,16 +1256,16 @@ static void __exit lynxfb_pci_remove(struct pci_dev * pdev)
 	}
 #ifdef CONFIG_MTRR
 	if(share->mtrr.vram_added)
-		mtrr_del(share->mtrr.vram,share->vidmem_start,share->vidmem_size);
+		mtrr_del(share->mtrr.vram, share->vidmem_start, share->vidmem_size);
 #endif
 	//	pci_release_regions(pdev);
 
 	iounmap(share->pvReg);
 	iounmap(share->pvMem);
-	spec_share = container_of(share,struct sm750_share,share);
+	spec_share = container_of(share, struct sm750_share, share);
 	kfree(g_settings);
 	kfree(spec_share);
-	pci_set_drvdata(pdev,NULL);
+	pci_set_drvdata(pdev, NULL);
 }
 
 static int __init lynxfb_setup(char * options)
@@ -1279,7 +1279,7 @@ static int __init lynxfb_setup(char * options)
 		return 0;
 	}
 
-	pr_info("options:%s\n",options);
+	pr_info("options:%s\n", options);
 
 	len = strlen(options) + 1;
 	g_settings = kzalloc(len, GFP_KERNEL);
@@ -1296,20 +1296,20 @@ static int __init lynxfb_setup(char * options)
 		strsep() updates @options to pointer after the first found token
 		it also returns the pointer ahead the token.
 		*/
-	while((opt = strsep(&options,":"))!=NULL)
+	while((opt = strsep(&options, ":"))!=NULL)
 	{
 		/* options that mean for any lynx chips are configured here */
-		if(!strncmp(opt,"noaccel",strlen("noaccel")))
+		if(!strncmp(opt, "noaccel", strlen("noaccel")))
 			g_noaccel = 1;
 #ifdef CONFIG_MTRR
-		else if(!strncmp(opt,"nomtrr",strlen("nomtrr")))
+		else if(!strncmp(opt, "nomtrr", strlen("nomtrr")))
 			g_nomtrr = 1;
 #endif
-		else if(!strncmp(opt,"dual",strlen("dual")))
+		else if(!strncmp(opt, "dual", strlen("dual")))
 			g_dualview = 1;
 		else
 		{
-			strcat(tmp,opt);
+			strcat(tmp, opt);
 			tmp += strlen(opt);
 			if(options != NULL)
 				*tmp++ = ':';
@@ -1319,7 +1319,7 @@ static int __init lynxfb_setup(char * options)
 	}
 
 	/* misc g_settings are transport to chip specific routines */
-	pr_info("parameter left for chip specific analysis:%s\n",g_settings);
+	pr_info("parameter left for chip specific analysis:%s\n", g_settings);
 	return 0;
 }
 
@@ -1328,7 +1328,7 @@ static struct pci_device_id smi_pci_table[] = {
 	{0,}
 };
 
-MODULE_DEVICE_TABLE(pci,smi_pci_table);
+MODULE_DEVICE_TABLE(pci, smi_pci_table);
 
 static struct pci_driver lynxfb_driver = {
 	.name =		"sm750fb",
@@ -1350,7 +1350,7 @@ static int __init lynxfb_init(void)
 #ifdef MODULE
 	option = g_option;
 #else
-	if(fb_get_options("sm750fb",&option))
+	if(fb_get_options("sm750fb", &option))
 		return -ENODEV;
 #endif
 
@@ -1366,7 +1366,7 @@ static void __exit lynxfb_exit(void)
 }
 module_exit(lynxfb_exit);
 
-module_param(g_option,charp,S_IRUGO);
+module_param(g_option, charp, S_IRUGO);
 
 MODULE_PARM_DESC(g_option,
 		 "\n\t\tCommon options:\n"
-- 
2.3.3


^ permalink raw reply related

* [PATCH v2 01/12] staging: sm750: Reindent
From: Michel von Czettritz @ 2015-03-26 22:24 UTC (permalink / raw)
  To: linux-fbdev

The reindent fixes the
"WARNING: please, no spaces at the start of a line",
"ERROR: code indent should use tabs where possible"
and some other small checkpatch.pl warnings and errors.

Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>

---
v2:
* Rebased on top of staging-testing
* fix function param. indent
---
 drivers/staging/sm750fb/sm750.c | 985 ++++++++++++++++++++--------------------
 1 file changed, 491 insertions(+), 494 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 4074375..87b50fc 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -31,12 +31,12 @@ int smi_indent = 0;
 
 
 /*
-#ifdef __BIG_ENDIAN
-ssize_t lynxfb_ops_write(struct fb_info *info, const char __user *buf,
- 	    size_t count, loff_t *ppos);
-ssize_t lynxfb_ops_read(struct fb_info *info, char __user *buf,
-			   size_t count, loff_t *ppos);
-#endif
+ * #ifdef __BIG_ENDIAN
+ * ssize_t lynxfb_ops_write(struct fb_info *info, const char __user *buf,
+ * size_t count, loff_t *ppos);
+ * ssize_t lynxfb_ops_read(struct fb_info *info, char __user *buf,
+ * size_t count, loff_t *ppos);
+ * #endif
  */
 
 typedef void (*PROC_SPEC_SETUP)(struct lynx_share*,char *);
@@ -57,56 +57,56 @@ static int g_dualview;
 static char * g_option = NULL;
 
 static const struct fb_videomode lynx750_ext[] = {
-	/*  	1024x600-60 VESA 	[1.71:1]	*/
+	/*	1024x600-60 VESA	[1.71:1]	*/
 	{NULL,  60, 1024, 600, 20423, 144,  40, 18, 1, 104, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/* 	1024x600-70 VESA */
+	/*	1024x600-70 VESA */
 	{NULL,  70, 1024, 600, 17211, 152,  48, 21, 1, 104, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*  	1024x600-75 VESA */
+	/*	1024x600-75 VESA */
 	{NULL,  75, 1024, 600, 15822, 160,  56, 23, 1, 104, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/*  	1024x600-85 VESA */
+	/*	1024x600-85 VESA */
 	{NULL,  85, 1024, 600, 13730, 168,  56, 26, 1, 112, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
 	/*	720x480	*/
 	{NULL, 60,  720,  480,  37427, 88,   16, 13, 1,   72,  3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
 	/*	1280x720		[1.78:1]	*/
 	{NULL, 60,  1280,  720,  13426, 162, 86, 22, 1,  136, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
 
-	/* 1280x768@60 */
+	/*	1280x768@60 */
 	{NULL,60,1280,768,12579,192,64,20,3,128,7,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
 
 	{NULL,60,1360,768,11804,208,64,23,1,144,3,
-	FB_SYNC_HOR_HIGH_ACT|FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT|FB_VMODE_NONINTERLACED},
 
 	/*	1360 x 768	[1.77083:1]	*/
 	{NULL,  60, 1360, 768, 11804, 208,  64, 23, 1, 144, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
 	/*	1368 x 768      [1.78:1]	*/
 	{NULL, 60,  1368,  768,  11647, 216, 72, 23, 1,  144, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
-	/* 	1440 x 900		[16:10]	*/
+	/*	1440 x 900		[16:10]	*/
 	{NULL, 60, 1440, 900, 9392, 232, 80, 28, 1, 152, 3,
-	FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
 
 	/*	1440x960		[15:10]	*/
 	{NULL, 60, 1440, 960, 8733, 240, 88, 30, 1, 152, 3,
-	FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
+	 FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT, FB_VMODE_NONINTERLACED},
 
 	/*	1920x1080	[16:9]	*/
 	{NULL, 60, 1920, 1080, 6734, 148, 88, 41, 1, 44, 3,
-	FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
+	 FB_SYNC_VERT_HIGH_ACT,FB_VMODE_NONINTERLACED},
 };
 
 
@@ -124,8 +124,8 @@ static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
 	cursor = &crtc->cursor;
 
 	if(fbcursor->image.width > cursor->maxW ||
-		fbcursor->image.height > cursor->maxH ||
-		 fbcursor->image.depth > 1){
+	   fbcursor->image.height > cursor->maxH ||
+	   fbcursor->image.depth > 1){
 		return -ENXIO;
 	}
 
@@ -136,19 +136,19 @@ static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
 
 	if(fbcursor->set & FB_CUR_SETPOS){
 		cursor->setPos(cursor,fbcursor->image.dx - info->var.xoffset,
-								fbcursor->image.dy - info->var.yoffset);
+			       fbcursor->image.dy - info->var.yoffset);
 	}
 
 	if(fbcursor->set & FB_CUR_SETCMAP){
 		/* get the 16bit color of kernel means */
 		u16 fg,bg;
 		fg = ((info->cmap.red[fbcursor->image.fg_color] & 0xf800))|
-				((info->cmap.green[fbcursor->image.fg_color] & 0xfc00) >> 5)|
-				((info->cmap.blue[fbcursor->image.fg_color] & 0xf800) >> 11);
+		      ((info->cmap.green[fbcursor->image.fg_color] & 0xfc00) >> 5)|
+		      ((info->cmap.blue[fbcursor->image.fg_color] & 0xf800) >> 11);
 
 		bg = ((info->cmap.red[fbcursor->image.bg_color] & 0xf800))|
-				((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5)|
-				((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
+		      ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5)|
+		      ((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
 
 		cursor->setColor(cursor,fg,bg);
 	}
@@ -157,9 +157,9 @@ static int lynxfb_ops_cursor(struct fb_info* info,struct fb_cursor* fbcursor)
 	if(fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE))
 	{
 		cursor->setData(cursor,
-						fbcursor->rop,
-						fbcursor->image.data,
-						fbcursor->mask);
+				fbcursor->rop,
+				fbcursor->image.data,
+				fbcursor->mask);
 	}
 
 	if(fbcursor->enable){
@@ -200,11 +200,10 @@ static void lynxfb_ops_fillrect(struct fb_info* info,const struct fb_fillrect* r
 		spin_lock(&share->slock);
 
 	share->accel.de_fillrect(&share->accel,
-							base,pitch,Bpp,
-							region->dx,region->dy,
-							region->width,region->height,
-							color,rop);
-
+				 base,pitch,Bpp,
+				 region->dx,region->dy,
+				 region->width,region->height,
+				 color,rop);
 	if (share->dual)
 		spin_unlock(&share->slock);
 }
@@ -232,10 +231,9 @@ static void lynxfb_ops_copyarea(struct fb_info * info,const struct fb_copyarea *
 		spin_lock(&share->slock);
 
 	share->accel.de_copyarea(&share->accel,
-							base,pitch,region->sx,region->sy,
-							base,pitch,Bpp,region->dx,region->dy,
-							region->width,region->height,HW_ROP2_COPY);
-
+				 base,pitch,region->sx,region->sy,
+				 base,pitch,Bpp,region->dx,region->dy,
+				 region->width,region->height,HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
 }
@@ -257,7 +255,7 @@ static void lynxfb_ops_imageblit(struct fb_info*info,const struct fb_image* imag
 
 	if(image->depth = 1){
 		if(info->fix.visual = FB_VISUAL_TRUECOLOR ||
-			info->fix.visual = FB_VISUAL_DIRECTCOLOR)
+		   info->fix.visual = FB_VISUAL_DIRECTCOLOR)
 		{
 			fgcol = ((u32*)info->pseudo_palette)[image->fg_color];
 			bgcol = ((u32*)info->pseudo_palette)[image->bg_color];
@@ -279,33 +277,32 @@ _do_work:
 		spin_lock(&share->slock);
 
 	share->accel.de_imageblit(&share->accel,
-					image->data,image->width>>3,0,
-					base,pitch,Bpp,
-					image->dx,image->dy,
-					image->width,image->height,
-					fgcol,bgcol,HW_ROP2_COPY);
-
+				  image->data,image->width>>3,0,
+				  base,pitch,Bpp,
+				  image->dx,image->dy,
+				  image->width,image->height,
+				  fgcol,bgcol,HW_ROP2_COPY);
 	if (share->dual)
 		spin_unlock(&share->slock);
 }
 
 static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
-        struct fb_info *info)
+				  struct fb_info *info)
 {
-    struct lynxfb_par * par;
-    struct lynxfb_crtc * crtc;
-    int ret;
-    
+	struct lynxfb_par * par;
+	struct lynxfb_crtc * crtc;
+	int ret;
 
-    if(!info)
-        return -EINVAL;
 
-    ret = 0;
-    par = info->par;
-    crtc = &par->crtc;
-    ret = crtc->proc_panDisplay(crtc, var, info);
+	if(!info)
+		return -EINVAL;
 
-    return ret;
+	ret = 0;
+	par = info->par;
+	crtc = &par->crtc;
+	ret = crtc->proc_panDisplay(crtc, var, info);
+
+	return ret;
 }
 
 static int lynxfb_ops_set_par(struct fb_info * info)
@@ -460,7 +457,7 @@ static int lynxfb_resume(struct pci_dev* pdev)
 	struct lynx_cursor * cursor;
 
 	int ret;
-	
+
 
 	ret = 0;
 	share = pci_get_drvdata(pdev);
@@ -526,7 +523,7 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo* var,struct fb_info* in
 	int ret;
 	resource_size_t request;
 
-	
+
 	par = info->par;
 	crtc = &par->crtc;
 	output = &par->output;
@@ -534,9 +531,9 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo* var,struct fb_info* in
 	ret = 0;
 
 	pr_debug("check var:%dx%d-%d\n",
-			var->xres,
-			var->yres,
-			var->bits_per_pixel);
+		 var->xres,
+		 var->yres,
+		 var->bits_per_pixel);
 
 
 	switch(var->bits_per_pixel){
@@ -611,64 +608,64 @@ exit:
 
 
 static int lynxfb_ops_setcolreg(unsigned regno,unsigned red,
-									unsigned green,unsigned blue,
-									unsigned transp,struct fb_info * info)
+				unsigned green,unsigned blue,
+				unsigned transp,struct fb_info * info)
 {
-    struct lynxfb_par * par;
-    struct lynxfb_crtc * crtc;
-    struct fb_var_screeninfo * var;
-    int ret;
-
-    par = info->par;
-    crtc = &par->crtc;
-    var = &info->var;
-    ret = 0;
-
-    //pr_debug("regno=%d,red=%d,green=%d,blue=%d\n",regno,red,green,blue);
-    if(regno > 256){
-        pr_err("regno = %d\n",regno);
-        return -EINVAL;
-    }
-
-    if(info->var.grayscale)
-        red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
-
-    if(var->bits_per_pixel = 8 && info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
-    {
-        red >>= 8;
-        green >>= 8;
-        blue >>= 8;
-        ret = crtc->proc_setColReg(crtc,regno,red,green,blue);
-        goto exit;
-    }
-
-
-    if(info->fix.visual = FB_VISUAL_TRUECOLOR && regno < 256 )
-    {
-        u32 val;
-        if(var->bits_per_pixel = 16 ||
-                var->bits_per_pixel = 32 ||
-                var->bits_per_pixel = 24)
-        {
-            val = chan_to_field(red,&var->red);
-            val |= chan_to_field(green,&var->green);
-            val |= chan_to_field(blue,&var->blue);
-            par->pseudo_palette[regno] = val;
-            goto exit;
-        }
-    }
-
-    ret = -EINVAL;
+	struct lynxfb_par * par;
+	struct lynxfb_crtc * crtc;
+	struct fb_var_screeninfo * var;
+	int ret;
+
+	par = info->par;
+	crtc = &par->crtc;
+	var = &info->var;
+	ret = 0;
+
+	//pr_debug("regno=%d,red=%d,green=%d,blue=%d\n",regno,red,green,blue);
+	if(regno > 256){
+		pr_err("regno = %d\n",regno);
+		return -EINVAL;
+	}
+
+	if(info->var.grayscale)
+		red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+
+	if(var->bits_per_pixel = 8 && info->fix.visual = FB_VISUAL_PSEUDOCOLOR)
+	{
+		red >>= 8;
+		green >>= 8;
+		blue >>= 8;
+		ret = crtc->proc_setColReg(crtc,regno,red,green,blue);
+		goto exit;
+	}
+
+
+	if(info->fix.visual = FB_VISUAL_TRUECOLOR && regno < 256 )
+	{
+		u32 val;
+		if(var->bits_per_pixel = 16 ||
+		   var->bits_per_pixel = 32 ||
+		   var->bits_per_pixel = 24)
+		{
+			val = chan_to_field(red,&var->red);
+			val |= chan_to_field(green,&var->green);
+			val |= chan_to_field(blue,&var->blue);
+			par->pseudo_palette[regno] = val;
+			goto exit;
+		}
+	}
+
+	ret = -EINVAL;
 
 exit:
-    return ret;
+	return ret;
 }
 
 static int lynxfb_ops_blank(int blank,struct fb_info* info)
 {
 	struct lynxfb_par * par;
 	struct lynxfb_output * output;
-	
+
 	pr_debug("blank = %d.\n",blank);
 	par = info->par;
 	output = &par->output;
@@ -677,89 +674,89 @@ static int lynxfb_ops_blank(int blank,struct fb_info* info)
 
 static int sm750fb_set_drv(struct lynxfb_par * par)
 {
-    int ret;
-    struct lynx_share * share;
-    struct sm750_share * spec_share;
-    struct lynxfb_output * output;
-    struct lynxfb_crtc * crtc;
-    
-    ret = 0;
-
-    share = par->share;
-    spec_share = container_of(share,struct sm750_share,share);
-    output = &par->output;
-    crtc = &par->crtc;
-
-    crtc->vidmem_size = (share->dual)?share->vidmem_size>>1:share->vidmem_size;
-    /* setup crtc and output member */
-    spec_share->hwCursor = g_hwcursor;
-
-    crtc->proc_setMode = hw_sm750_crtc_setMode;
-    crtc->proc_checkMode = hw_sm750_crtc_checkMode;
-    crtc->proc_setColReg = hw_sm750_setColReg;
-    crtc->proc_panDisplay = hw_sm750_pan_display;
-    crtc->clear = hw_sm750_crtc_clear;
-    crtc->line_pad = 16;
-    //crtc->xpanstep = crtc->ypanstep = crtc->ywrapstep = 0;
-    crtc->xpanstep = 8;
-    crtc->ypanstep = 1;
-    crtc->ywrapstep = 0;
-
-    output->proc_setMode = hw_sm750_output_setMode;
-    output->proc_checkMode = hw_sm750_output_checkMode;
-
-    output->proc_setBLANK = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_setBLANK:hw_sm750_setBLANK;
-    output->clear = hw_sm750_output_clear;
-    /* chip specific phase */
-    share->accel.de_wait = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_deWait: hw_sm750_deWait;
-    switch (spec_share->state.dataflow)
-    {
-        case sm750_simul_pri:
-            output->paths = sm750_pnc;
-            crtc->channel = sm750_primary;
-            crtc->oScreen = 0;
-            crtc->vScreen = share->pvMem;
-            pr_info("use simul primary mode\n");
-            break;
-        case sm750_simul_sec:
-            output->paths = sm750_pnc;
-            crtc->channel = sm750_secondary;
-            crtc->oScreen = 0;
-            crtc->vScreen = share->pvMem;
-            break;
-        case sm750_dual_normal:
-            if(par->index = 0){
-                output->paths = sm750_panel;
-                crtc->channel = sm750_primary;
-                crtc->oScreen = 0;
-                crtc->vScreen = share->pvMem;
-            }else{
-                output->paths = sm750_crt;
-                crtc->channel = sm750_secondary;
-                /* not consider of padding stuffs for oScreen,need fix*/
-                crtc->oScreen = (share->vidmem_size >> 1);
-                crtc->vScreen = share->pvMem + crtc->oScreen;
-            }
-            break;
-        case sm750_dual_swap:
-            if(par->index = 0){
-                output->paths = sm750_panel;
-                crtc->channel = sm750_secondary;
-                crtc->oScreen = 0;
-                crtc->vScreen = share->pvMem;
-            }else{
-                output->paths = sm750_crt;
-                crtc->channel = sm750_primary;
-                /* not consider of padding stuffs for oScreen,need fix*/
-                crtc->oScreen = (share->vidmem_size >> 1);
-                crtc->vScreen = share->pvMem + crtc->oScreen;
-            }
-            break;
-        default:
-            ret = -EINVAL;
-    }
-
-    return ret;
+	int ret;
+	struct lynx_share * share;
+	struct sm750_share * spec_share;
+	struct lynxfb_output * output;
+	struct lynxfb_crtc * crtc;
+
+	ret = 0;
+
+	share = par->share;
+	spec_share = container_of(share,struct sm750_share,share);
+	output = &par->output;
+	crtc = &par->crtc;
+
+	crtc->vidmem_size = (share->dual)?share->vidmem_size>>1:share->vidmem_size;
+	/* setup crtc and output member */
+	spec_share->hwCursor = g_hwcursor;
+
+	crtc->proc_setMode = hw_sm750_crtc_setMode;
+	crtc->proc_checkMode = hw_sm750_crtc_checkMode;
+	crtc->proc_setColReg = hw_sm750_setColReg;
+	crtc->proc_panDisplay = hw_sm750_pan_display;
+	crtc->clear = hw_sm750_crtc_clear;
+	crtc->line_pad = 16;
+	//crtc->xpanstep = crtc->ypanstep = crtc->ywrapstep = 0;
+	crtc->xpanstep = 8;
+	crtc->ypanstep = 1;
+	crtc->ywrapstep = 0;
+
+	output->proc_setMode = hw_sm750_output_setMode;
+	output->proc_checkMode = hw_sm750_output_checkMode;
+
+	output->proc_setBLANK = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_setBLANK:hw_sm750_setBLANK;
+	output->clear = hw_sm750_output_clear;
+	/* chip specific phase */
+	share->accel.de_wait = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_deWait: hw_sm750_deWait;
+	switch (spec_share->state.dataflow)
+	{
+	case sm750_simul_pri:
+		output->paths = sm750_pnc;
+		crtc->channel = sm750_primary;
+		crtc->oScreen = 0;
+		crtc->vScreen = share->pvMem;
+		pr_info("use simul primary mode\n");
+		break;
+	case sm750_simul_sec:
+		output->paths = sm750_pnc;
+		crtc->channel = sm750_secondary;
+		crtc->oScreen = 0;
+		crtc->vScreen = share->pvMem;
+		break;
+	case sm750_dual_normal:
+		if(par->index = 0){
+			output->paths = sm750_panel;
+			crtc->channel = sm750_primary;
+			crtc->oScreen = 0;
+			crtc->vScreen = share->pvMem;
+		}else{
+			output->paths = sm750_crt;
+			crtc->channel = sm750_secondary;
+			/* not consider of padding stuffs for oScreen,need fix*/
+			crtc->oScreen = (share->vidmem_size >> 1);
+			crtc->vScreen = share->pvMem + crtc->oScreen;
+		}
+		break;
+	case sm750_dual_swap:
+		if(par->index = 0){
+			output->paths = sm750_panel;
+			crtc->channel = sm750_secondary;
+			crtc->oScreen = 0;
+			crtc->vScreen = share->pvMem;
+		}else{
+			output->paths = sm750_crt;
+			crtc->channel = sm750_primary;
+			/* not consider of padding stuffs for oScreen,need fix*/
+			crtc->oScreen = (share->vidmem_size >> 1);
+			crtc->vScreen = share->pvMem + crtc->oScreen;
+		}
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
 }
 
 static struct fb_ops lynxfb_ops={
@@ -778,106 +775,106 @@ static struct fb_ops lynxfb_ops={
 
 static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 {
-    int i;
-    struct lynxfb_par * par;
-    struct lynx_share * share;
-    struct lynxfb_crtc * crtc;
-    struct lynxfb_output * output;
-    struct fb_var_screeninfo * var;
-    struct fb_fix_screeninfo * fix;
-
-    const struct fb_videomode * pdb[] = {
-        lynx750_ext, NULL,vesa_modes,
-    };
-    int cdb[] = {ARRAY_SIZE(lynx750_ext),0,VESA_MODEDB_SIZE};
-    static const char * mdb_desc[] ={
-        "driver prepared modes",
-        "kernel prepared default modedb",
-        "kernel HELPERS prepared vesa_modes",
-    };
-
-
-    static const char * fixId[2]-    {
-        "sm750_fb1","sm750_fb2",
-    };
-
-    int ret,line_length;
-    
-    ret = 0;
-    par = (struct lynxfb_par *)info->par;
-    share = par->share;
-    crtc = &par->crtc;
-    output = &par->output;
-    var = &info->var;
-    fix = &info->fix;
-
-    /* set index */
-    par->index = index;
-    output->channel = &crtc->channel;
+	int i;
+	struct lynxfb_par * par;
+	struct lynx_share * share;
+	struct lynxfb_crtc * crtc;
+	struct lynxfb_output * output;
+	struct fb_var_screeninfo * var;
+	struct fb_fix_screeninfo * fix;
+
+	const struct fb_videomode * pdb[] = {
+		lynx750_ext, NULL,vesa_modes,
+	};
+	int cdb[] = {ARRAY_SIZE(lynx750_ext),0,VESA_MODEDB_SIZE};
+	static const char * mdb_desc[] ={
+		"driver prepared modes",
+		"kernel prepared default modedb",
+		"kernel HELPERS prepared vesa_modes",
+	};
+
+
+	static const char * fixId[2]+	{
+		"sm750_fb1","sm750_fb2",
+	};
+
+	int ret,line_length;
+
+	ret = 0;
+	par = (struct lynxfb_par *)info->par;
+	share = par->share;
+	crtc = &par->crtc;
+	output = &par->output;
+	var = &info->var;
+	fix = &info->fix;
+
+	/* set index */
+	par->index = index;
+	output->channel = &crtc->channel;
 	sm750fb_set_drv(par);
-        lynxfb_ops.fb_pan_display = lynxfb_ops_pan_display;
-
-
-    /* set current cursor variable and proc pointer,
-     * must be set after crtc member initialized */
-       crtc->cursor.offset = crtc->oScreen + crtc->vidmem_size - 1024;
-        crtc->cursor.mmio = share->pvReg + 0x800f0 + (int)crtc->channel * 0x140;
-
-        pr_info("crtc->cursor.mmio = %p\n",crtc->cursor.mmio);
-        crtc->cursor.maxH = crtc->cursor.maxW = 64;
-        crtc->cursor.size = crtc->cursor.maxH*crtc->cursor.maxW*2/8;
-        crtc->cursor.disable = hw_cursor_disable;
-        crtc->cursor.enable = hw_cursor_enable;
-        crtc->cursor.setColor = hw_cursor_setColor;
-        crtc->cursor.setPos = hw_cursor_setPos;
-        crtc->cursor.setSize = hw_cursor_setSize;
-        crtc->cursor.setData = hw_cursor_setData;
-        crtc->cursor.vstart = share->pvMem + crtc->cursor.offset;
-
-
-    crtc->cursor.share = share;
-	memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
-    if(!g_hwcursor){
-        lynxfb_ops.fb_cursor = NULL;
-        crtc->cursor.disable(&crtc->cursor);
-    }
-
-
-    /* set info->fbops, must be set before fb_find_mode */
-    if(!share->accel_off){
-        /* use 2d acceleration */
-        lynxfb_ops.fb_fillrect = lynxfb_ops_fillrect;
-        lynxfb_ops.fb_copyarea = lynxfb_ops_copyarea;
-        lynxfb_ops.fb_imageblit = lynxfb_ops_imageblit;
-    }
-    info->fbops = &lynxfb_ops;
-
-    if(!g_fbmode[index]){
-        g_fbmode[index] = g_def_fbmode;
-        if(index)
-            g_fbmode[index] = g_fbmode[0];
-    }
+	lynxfb_ops.fb_pan_display = lynxfb_ops_pan_display;
+
+
+	/* set current cursor variable and proc pointer,
+	 * must be set after crtc member initialized */
+	crtc->cursor.offset = crtc->oScreen + crtc->vidmem_size - 1024;
+	crtc->cursor.mmio = share->pvReg + 0x800f0 + (int)crtc->channel * 0x140;
+
+	pr_info("crtc->cursor.mmio = %p\n",crtc->cursor.mmio);
+	crtc->cursor.maxH = crtc->cursor.maxW = 64;
+	crtc->cursor.size = crtc->cursor.maxH*crtc->cursor.maxW*2/8;
+	crtc->cursor.disable = hw_cursor_disable;
+	crtc->cursor.enable = hw_cursor_enable;
+	crtc->cursor.setColor = hw_cursor_setColor;
+	crtc->cursor.setPos = hw_cursor_setPos;
+	crtc->cursor.setSize = hw_cursor_setSize;
+	crtc->cursor.setData = hw_cursor_setData;
+	crtc->cursor.vstart = share->pvMem + crtc->cursor.offset;
+
+
+	crtc->cursor.share = share;
+		memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
+	if(!g_hwcursor){
+		lynxfb_ops.fb_cursor = NULL;
+		crtc->cursor.disable(&crtc->cursor);
+	}
+
+
+	/* set info->fbops, must be set before fb_find_mode */
+	if(!share->accel_off){
+		/* use 2d acceleration */
+		lynxfb_ops.fb_fillrect = lynxfb_ops_fillrect;
+		lynxfb_ops.fb_copyarea = lynxfb_ops_copyarea;
+		lynxfb_ops.fb_imageblit = lynxfb_ops_imageblit;
+	}
+	info->fbops = &lynxfb_ops;
+
+	if(!g_fbmode[index]){
+		g_fbmode[index] = g_def_fbmode;
+		if(index)
+			g_fbmode[index] = g_fbmode[0];
+	}
 
 
 	for(i=0;i<3;i++){
 
 		ret = fb_find_mode(var,info,g_fbmode[index],
-				pdb[i],cdb[i],NULL,8);
+				   pdb[i],cdb[i],NULL,8);
 
 		if(ret = 1){
 			pr_info("success! use specified mode:%s in %s\n",
-					g_fbmode[index],
-					mdb_desc[i]);
+				g_fbmode[index],
+				mdb_desc[i]);
 			break;
 		}else if(ret = 2){
 			pr_warn("use specified mode:%s in %s,with an ignored refresh rate\n",
-					g_fbmode[index],
-					mdb_desc[i]);
+				g_fbmode[index],
+				mdb_desc[i]);
 			break;
 		}else if(ret = 3){
 			pr_warn("wanna use default mode\n");
-//			break;
+			//			break;
 		}else if(ret = 4){
 			pr_warn("fall back to any valid mode\n");
 		}else{
@@ -885,97 +882,97 @@ static int lynxfb_set_fbinfo(struct fb_info* info,int index)
 		}
 	}
 
-    /* some member of info->var had been set by fb_find_mode */
-
-    pr_info("Member of info->var is :\n\
-            xres=%d\n\
-            yres=%d\n\
-            xres_virtual=%d\n\
-            yres_virtual=%d\n\
-            xoffset=%d\n\
-            yoffset=%d\n\
-            bits_per_pixel=%d\n \
-            ...\n",var->xres,var->yres,var->xres_virtual,var->yres_virtual,
-            var->xoffset,var->yoffset,var->bits_per_pixel);
-
-    /* set par */
-    par->info = info;
-
-    /* set info */
-    line_length = PADDING(crtc->line_pad,
-            (var->xres_virtual * var->bits_per_pixel/8));
-
-    info->pseudo_palette = &par->pseudo_palette[0];
-    info->screen_base = crtc->vScreen;
-    pr_debug("screen_base vaddr = %p\n",info->screen_base);
+	/* some member of info->var had been set by fb_find_mode */
+
+	pr_info("Member of info->var is :\n\
+			xres=%d\n\
+			yres=%d\n\
+			xres_virtual=%d\n\
+			yres_virtual=%d\n\
+			xoffset=%d\n\
+			yoffset=%d\n\
+			bits_per_pixel=%d\n \
+			...\n",var->xres,var->yres,var->xres_virtual,var->yres_virtual,
+			var->xoffset,var->yoffset,var->bits_per_pixel);
+
+	/* set par */
+	par->info = info;
+
+	/* set info */
+	line_length = PADDING(crtc->line_pad,
+			      (var->xres_virtual * var->bits_per_pixel/8));
+
+	info->pseudo_palette = &par->pseudo_palette[0];
+	info->screen_base = crtc->vScreen;
+	pr_debug("screen_base vaddr = %p\n",info->screen_base);
 	info->screen_size = line_length * var->yres_virtual;
 	info->flags = FBINFO_FLAG_DEFAULT|0;
 
-    /* set info->fix */
-    fix->type = FB_TYPE_PACKED_PIXELS;
-    fix->type_aux = 0;
-    fix->xpanstep = crtc->xpanstep;
-    fix->ypanstep = crtc->ypanstep;
-    fix->ywrapstep = crtc->ywrapstep;
-    fix->accel = FB_ACCEL_SMI;
+	/* set info->fix */
+	fix->type = FB_TYPE_PACKED_PIXELS;
+	fix->type_aux = 0;
+	fix->xpanstep = crtc->xpanstep;
+	fix->ypanstep = crtc->ypanstep;
+	fix->ywrapstep = crtc->ywrapstep;
+	fix->accel = FB_ACCEL_SMI;
 
-    strlcpy(fix->id,fixId[index],sizeof(fix->id));
+	strlcpy(fix->id,fixId[index],sizeof(fix->id));
 
 
 	fix->smem_start = crtc->oScreen + share->vidmem_start;
 	pr_info("fix->smem_start = %lx\n",fix->smem_start);
-    /* according to mmap experiment from user space application,
-     * fix->mmio_len should not larger than virtual size
-     * (xres_virtual x yres_virtual x ByPP)
-     * Below line maybe buggy when user mmap fb dev node and write
-     * data into the bound over virtual size
-     * */
-    fix->smem_len = crtc->vidmem_size;
-    pr_info("fix->smem_len = %x\n",fix->smem_len);
-    info->screen_size = fix->smem_len;
-    fix->line_length = line_length;
-    fix->mmio_start = share->vidreg_start;
-    pr_info("fix->mmio_start = %lx\n",fix->mmio_start);
-    fix->mmio_len = share->vidreg_size;
-    pr_info("fix->mmio_len = %x\n",fix->mmio_len);
-    switch(var->bits_per_pixel)
-    {
-        case 8:
-            fix->visual = FB_VISUAL_PSEUDOCOLOR;
-            break;
-        case 16:
-        case 32:
-            fix->visual = FB_VISUAL_TRUECOLOR;
-            break;
-    }
-
-    /* set var */
-    var->activate = FB_ACTIVATE_NOW;
-    var->accel_flags = 0;
-    var->vmode = FB_VMODE_NONINTERLACED;
-
-    pr_debug("#1 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
-            info->cmap.start,info->cmap.len,
-            info->cmap.red,info->cmap.green,info->cmap.blue,
-            info->cmap.transp);
-
-    if((ret = fb_alloc_cmap(&info->cmap,256,0)) < 0){
-        pr_err("Could not allcate memory for cmap.\n");
-        goto exit;
-    }
-
-    pr_debug("#2 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
-            info->cmap.start,info->cmap.len,
-            info->cmap.red,info->cmap.green,info->cmap.blue,
-            info->cmap.transp);
+	/* according to mmap experiment from user space application,
+	 * fix->mmio_len should not larger than virtual size
+	 * (xres_virtual x yres_virtual x ByPP)
+	 * Below line maybe buggy when user mmap fb dev node and write
+	 * data into the bound over virtual size
+	 * */
+	fix->smem_len = crtc->vidmem_size;
+	pr_info("fix->smem_len = %x\n",fix->smem_len);
+	info->screen_size = fix->smem_len;
+	fix->line_length = line_length;
+	fix->mmio_start = share->vidreg_start;
+	pr_info("fix->mmio_start = %lx\n",fix->mmio_start);
+	fix->mmio_len = share->vidreg_size;
+	pr_info("fix->mmio_len = %x\n",fix->mmio_len);
+	switch(var->bits_per_pixel)
+	{
+	case 8:
+		fix->visual = FB_VISUAL_PSEUDOCOLOR;
+		break;
+	case 16:
+	case 32:
+		fix->visual = FB_VISUAL_TRUECOLOR;
+		break;
+	}
+
+	/* set var */
+	var->activate = FB_ACTIVATE_NOW;
+	var->accel_flags = 0;
+	var->vmode = FB_VMODE_NONINTERLACED;
+
+	pr_debug("#1 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
+		 info->cmap.start,info->cmap.len,
+		 info->cmap.red,info->cmap.green,info->cmap.blue,
+		 info->cmap.transp);
+
+	if((ret = fb_alloc_cmap(&info->cmap,256,0)) < 0){
+		pr_err("Could not allcate memory for cmap.\n");
+		goto exit;
+	}
+
+	pr_debug("#2 show info->cmap : \nstart=%d,len=%d,red=%p,green=%p,blue=%p,transp=%p\n",
+		 info->cmap.start,info->cmap.len,
+		 info->cmap.red,info->cmap.green,info->cmap.blue,
+		 info->cmap.transp);
 
 exit:
 	lynxfb_ops_check_var(var,info);
-//    lynxfb_ops_set_par(info);
-    return ret;
+	//    lynxfb_ops_set_par(info);
+	return ret;
 }
 
-/* 	chip specific g_option configuration routine */
+/*	chip specific g_option configuration routine */
 static void sm750fb_setup(struct lynx_share * share,char * src)
 {
 	struct sm750_share * spec_share;
@@ -984,98 +981,98 @@ static void sm750fb_setup(struct lynx_share * share,char * src)
 	char * exp_res;
 #endif
 	int swap;
-	
+
 
 	spec_share = container_of(share,struct sm750_share,share);
 #ifdef CAP_EXPENSIION
-        exp_res = NULL;
+	exp_res = NULL;
 #endif
-        swap = 0;
-
-        spec_share->state.initParm.chip_clk = 0;
-        spec_share->state.initParm.mem_clk = 0;
-        spec_share->state.initParm.master_clk = 0;
-        spec_share->state.initParm.powerMode = 0;
-        spec_share->state.initParm.setAllEngOff = 0;
-        spec_share->state.initParm.resetMemory = 1;
-
-        /*defaultly turn g_hwcursor on for both view */
-        g_hwcursor = 3;
-
-        if(!src || !*src){
-            pr_warn("no specific g_option.\n");
-            goto NO_PARAM;
-        }
-
-        while((opt = strsep(&src,":")) != NULL && *opt != 0){
-			pr_err("opt=%s\n",opt);
-			pr_err("src=%s\n",src);
-
-            if(!strncmp(opt,"swap",strlen("swap")))
-                swap = 1;
-            else if(!strncmp(opt,"nocrt",strlen("nocrt")))
-                spec_share->state.nocrt = 1;
-            else if(!strncmp(opt,"36bit",strlen("36bit")))
-                spec_share->state.pnltype = sm750_doubleTFT;
-            else if(!strncmp(opt,"18bit",strlen("18bit")))
-                spec_share->state.pnltype = sm750_dualTFT;
-            else if(!strncmp(opt,"24bit",strlen("24bit")))
-                spec_share->state.pnltype = sm750_24TFT;
+	swap = 0;
+
+	spec_share->state.initParm.chip_clk = 0;
+	spec_share->state.initParm.mem_clk = 0;
+	spec_share->state.initParm.master_clk = 0;
+	spec_share->state.initParm.powerMode = 0;
+	spec_share->state.initParm.setAllEngOff = 0;
+	spec_share->state.initParm.resetMemory = 1;
+
+	/*defaultly turn g_hwcursor on for both view */
+	g_hwcursor = 3;
+
+	if(!src || !*src){
+		pr_warn("no specific g_option.\n");
+		goto NO_PARAM;
+	}
+
+	while((opt = strsep(&src,":")) != NULL && *opt != 0){
+		pr_err("opt=%s\n",opt);
+		pr_err("src=%s\n",src);
+
+		if(!strncmp(opt,"swap",strlen("swap")))
+			swap = 1;
+		else if(!strncmp(opt,"nocrt",strlen("nocrt")))
+			spec_share->state.nocrt = 1;
+		else if(!strncmp(opt,"36bit",strlen("36bit")))
+			spec_share->state.pnltype = sm750_doubleTFT;
+		else if(!strncmp(opt,"18bit",strlen("18bit")))
+			spec_share->state.pnltype = sm750_dualTFT;
+		else if(!strncmp(opt,"24bit",strlen("24bit")))
+			spec_share->state.pnltype = sm750_24TFT;
 #ifdef CAP_EXPANSION
-            else if(!strncmp(opt,"exp:",strlen("exp:")))
-                exp_res = opt + strlen("exp:");
+		else if(!strncmp(opt,"exp:",strlen("exp:")))
+			exp_res = opt + strlen("exp:");
 #endif
-            else if(!strncmp(opt,"nohwc0",strlen("nohwc0")))
-                g_hwcursor &= ~0x1;
-            else if(!strncmp(opt,"nohwc1",strlen("nohwc1")))
-                g_hwcursor &= ~0x2;
-            else if(!strncmp(opt,"nohwc",strlen("nohwc")))
-                g_hwcursor = 0;
-            else
-            {
-                if(!g_fbmode[0]){
-                    g_fbmode[0] = opt;
-                    pr_info("find fbmode0 : %s\n",g_fbmode[0]);
-                }else if(!g_fbmode[1]){
-                    g_fbmode[1] = opt;
-                    pr_info("find fbmode1 : %s\n",g_fbmode[1]);
-                }else{
-                    pr_warn("How many view you wann set?\n");
-                }
-            }
-        }
+		else if(!strncmp(opt,"nohwc0",strlen("nohwc0")))
+			g_hwcursor &= ~0x1;
+		else if(!strncmp(opt,"nohwc1",strlen("nohwc1")))
+			g_hwcursor &= ~0x2;
+		else if(!strncmp(opt,"nohwc",strlen("nohwc")))
+			g_hwcursor = 0;
+		else
+		{
+			if(!g_fbmode[0]){
+				g_fbmode[0] = opt;
+				pr_info("find fbmode0 : %s\n",g_fbmode[0]);
+			}else if(!g_fbmode[1]){
+				g_fbmode[1] = opt;
+				pr_info("find fbmode1 : %s\n",g_fbmode[1]);
+			}else{
+				pr_warn("How many view you wann set?\n");
+			}
+		}
+	}
 #ifdef CAP_EXPANSION
-        if(getExpRes(exp_res,&spec_share->state.xLCD,&spec_share->state.yLCD))
-        {
-            /* seems exp_res is not valid*/
-            spec_share->state.xLCD = spec_share->state.yLCD = 0;
-        }
+	if(getExpRes(exp_res,&spec_share->state.xLCD,&spec_share->state.yLCD))
+	{
+		/* seems exp_res is not valid*/
+		spec_share->state.xLCD = spec_share->state.yLCD = 0;
+	}
 #endif
 
 NO_PARAM:
-        if(share->revid != SM750LE_REVISION_ID){
-            if(share->dual)
-            {
-                if(swap)
-                    spec_share->state.dataflow = sm750_dual_swap;
-                else
-                    spec_share->state.dataflow = sm750_dual_normal;
-            }else{
-                if(swap)
-                    spec_share->state.dataflow = sm750_simul_sec;
-                else
-                    spec_share->state.dataflow = sm750_simul_pri;
-            }
-        }else{
-            /* SM750LE only have one crt channel */
-            spec_share->state.dataflow = sm750_simul_sec;
-            /* sm750le do not have complex attributes*/
-            spec_share->state.nocrt = 0;
-        }
+	if(share->revid != SM750LE_REVISION_ID){
+		if(share->dual)
+		{
+			if(swap)
+				spec_share->state.dataflow = sm750_dual_swap;
+			else
+				spec_share->state.dataflow = sm750_dual_normal;
+		}else{
+			if(swap)
+				spec_share->state.dataflow = sm750_simul_sec;
+			else
+				spec_share->state.dataflow = sm750_simul_pri;
+		}
+	}else{
+		/* SM750LE only have one crt channel */
+		spec_share->state.dataflow = sm750_simul_sec;
+		/* sm750le do not have complex attributes*/
+		spec_share->state.nocrt = 0;
+	}
 }
 
 static int lynxfb_pci_probe(struct pci_dev * pdev,
-		const struct pci_device_id * ent)
+			    const struct pci_device_id * ent)
 {
 	struct fb_info * info[] = {NULL,NULL};
 	struct lynx_share * share = NULL;
@@ -1083,7 +1080,7 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	struct sm750_share *spec_share = NULL;
 	size_t spec_offset = 0;
 	int fbidx;
-	
+
 
 	/* enable device */
 	if(pci_enable_device(pdev)){
@@ -1145,8 +1142,8 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	if(!share->mtrr_off){
 		pr_info("enable mtrr\n");
 		share->mtrr.vram = mtrr_add(share->vidmem_start,
-				share->vidmem_size,
-				MTRR_TYPE_WRCOMB,1);
+					    share->vidmem_size,
+					    MTRR_TYPE_WRCOMB,1);
 
 		if(share->mtrr.vram < 0){
 			/* don't block driver with the failure of MTRR */
@@ -1168,52 +1165,52 @@ static int lynxfb_pci_probe(struct pci_dev * pdev,
 	hw_sm750_inithw(share,pdev);
 
 	/* allocate frame buffer info structor according to g_dualview */
-		fbidx = 0;
+	fbidx = 0;
 ALLOC_FB:
-		info[fbidx] = framebuffer_alloc(sizeof(struct lynxfb_par),&pdev->dev);
-		if(!info[fbidx])
-		{
-			pr_err("Could not allocate framebuffer #%d.\n",fbidx);
+	info[fbidx] = framebuffer_alloc(sizeof(struct lynxfb_par),&pdev->dev);
+	if(!info[fbidx])
+	{
+		pr_err("Could not allocate framebuffer #%d.\n",fbidx);
+		if(fbidx = 0)
+			goto err_info0_alloc;
+		else
+			goto err_info1_alloc;
+	}
+	else
+	{
+		struct lynxfb_par * par;
+		int errno;
+		pr_info("framebuffer #%d alloc okay\n",fbidx);
+		share->fbinfo[fbidx] = info[fbidx];
+		par = info[fbidx]->par;
+		par->share = share;
+
+		/* set fb_info structure */
+		if(lynxfb_set_fbinfo(info[fbidx],fbidx)){
+			pr_err("Failed to initial fb_info #%d.\n",fbidx);
 			if(fbidx = 0)
-				goto err_info0_alloc;
+				goto err_info0_set;
 			else
-				goto err_info1_alloc;
+				goto err_info1_set;
 		}
-		else
-		{
-			struct lynxfb_par * par;
-			int errno;
-			pr_info("framebuffer #%d alloc okay\n",fbidx);
-			share->fbinfo[fbidx] = info[fbidx];
-			par = info[fbidx]->par;
-			par->share = share;
-
-			/* set fb_info structure */
-			if(lynxfb_set_fbinfo(info[fbidx],fbidx)){
-				pr_err("Failed to initial fb_info #%d.\n",fbidx);
-				if(fbidx = 0)
-					goto err_info0_set;
-				else
-					goto err_info1_set;
-			}
 
-			/* register frame buffer*/
-			pr_info("Ready to register framebuffer #%d.\n",fbidx);
-			errno = register_framebuffer(info[fbidx]);
-			if (errno < 0) {
-				pr_err("Failed to register fb_info #%d. err %d\n",fbidx, errno);
-				if(fbidx = 0)
-					goto err_register0;
-				else
-					goto err_register1;
-			}
-			pr_info("Accomplished register framebuffer #%d.\n",fbidx);
+		/* register frame buffer*/
+		pr_info("Ready to register framebuffer #%d.\n",fbidx);
+		errno = register_framebuffer(info[fbidx]);
+		if (errno < 0) {
+			pr_err("Failed to register fb_info #%d. err %d\n",fbidx, errno);
+			if(fbidx = 0)
+				goto err_register0;
+			else
+				goto err_register1;
 		}
+		pr_info("Accomplished register framebuffer #%d.\n",fbidx);
+	}
 
-		/* no dual view by far */
-		fbidx++;
-		if(share->dual && fbidx < 2)
-			goto ALLOC_FB;
+	/* no dual view by far */
+	fbidx++;
+	if(share->dual && fbidx < 2)
+		goto ALLOC_FB;
 
 	return 0;
 
@@ -1275,7 +1272,7 @@ static int __init lynxfb_setup(char * options)
 {
 	int len;
 	char * opt,*tmp;
-	
+
 
 	if(!options || !*options){
 		pr_warn("no options.\n");
@@ -1291,7 +1288,7 @@ static int __init lynxfb_setup(char * options)
 
 	tmp = g_settings;
 
-	/* 	Notes:
+	/*	Notes:
 		char * strsep(char **s,const char * ct);
 		@s: the string to be searched
 		@ct :the characters to search for
@@ -1372,14 +1369,14 @@ module_exit(lynxfb_exit);
 module_param(g_option,charp,S_IRUGO);
 
 MODULE_PARM_DESC(g_option,
-		"\n\t\tCommon options:\n"
-		"\t\tnoaccel:disable 2d capabilities\n"
-		"\t\tnomtrr:disable MTRR attribute for video memory\n"
-		"\t\tdualview:dual frame buffer feature enabled\n"
-		"\t\tnohwc:disable hardware cursor\n"
-		"\t\tUsual example:\n"
-		"\t\tinsmod ./sm750fb.ko g_option=\"noaccel,nohwc,1280x1024-8@60\"\n"
-		);
+		 "\n\t\tCommon options:\n"
+		 "\t\tnoaccel:disable 2d capabilities\n"
+		 "\t\tnomtrr:disable MTRR attribute for video memory\n"
+		 "\t\tdualview:dual frame buffer feature enabled\n"
+		 "\t\tnohwc:disable hardware cursor\n"
+		 "\t\tUsual example:\n"
+		 "\t\tinsmod ./sm750fb.ko g_option=\"noaccel,nohwc,1280x1024-8@60\"\n"
+		 );
 
 MODULE_AUTHOR("monk liu <monk.liu@siliconmotion.com>");
 MODULE_AUTHOR("Sudip Mukherjee <sudip@vectorindia.org>");
-- 
2.3.3


^ permalink raw reply related

* [PATCH v2 00/12] reformat sm750.c
From: Michel von Czettritz @ 2015-03-26 22:24 UTC (permalink / raw)
  To: linux-fbdev

As suggested by Dan Carpenter, I've rewritten my patch as a series of smaller patches.

Dan's Mail: http://marc.info/?l=kernel-janitors&m\x142679651624193&w=2

Patches 1-11 only reformat code. Patch 12 changes the code, by moving assignments out of if conditions.
The module builds, but as I don't own a sm750 card, I can't test it.

---
v2:
* rebase on top of staging-testing
* delete original patch 6, as the original code was deleted by Lorenzo
* fix formatting as proposed by Dan Carpenter

Michel von Czettritz (12):
  staging: sm750: Reindent
  staging: sm750: add missing spaces after ','
  staging: sm750: move '{' to previous line
  staging: sm750: fix whitespace around braces
  staging: sm750: fix whitespaces around pointers
  staging: sm750: no braces for single stmt if blocks
  staging: sm750: comment formating
  staging: sm750: 80 characters per line
  staging: sm750: blank line after declaration
  staging: sm750: fix whitespace
  staging: sm750: fix whitspace for cast to pointer
  staging: sm750: move assignment out of if cond

 drivers/staging/sm750fb/sm750.c | 1358 ++++++++++++++++++++-------------------
 1 file changed, 687 insertions(+), 671 deletions(-)

-- 
2.3.3


^ permalink raw reply

* Re: [PATCH 02/13] staging: sm750: add missing spaces after ','
From: Dan Carpenter @ 2015-03-26 15:50 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150322231131.GA17080@x230-arch.club.entropia.de>

On Thu, Mar 26, 2015 at 04:28:12PM +0100, Michel von Czettritz wrote:
> >> ---
> >>  drivers/staging/sm750fb/sm750.c | 264 ++++++++++++++++++++--------------------
> >>  1 file changed, 132 insertions(+), 132 deletions(-)
> > 
> > As I didn't take your first patch, the rest of the patches in this
> > series do not apply.
> > 
> > Please fix them up, rebase on my latest staging-testing branch of
> > staging.git, and resend so that I can apply them.
> > 
> > thanks,
> > 
> > greg k-h
> 
> I am, as a newbie, a bit confused. 
> Should i resend my fixes with RESEND or with a v2 flag?

Send a v2 and under the --- cut off write:
---
v2: rebased on top of latest staging-testing branch.


> As Sudip i applied Lorenzos conflicting patches on statging-testing and rebased my patches on them. 

I don't understand this sentence.

> Should i wait until Lorenzos patches are applied?

If your patch doesn't apply when Greg gets to it, then you'll have to
redo it.  Figure out how to make it work.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH 02/13] staging: sm750: add missing spaces after ','
From: Michel von Czettritz @ 2015-03-26 15:28 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150322231131.GA17080@x230-arch.club.entropia.de>

On 03/26/2015 01:06 PM, Greg KH wrote:
> On Mon, Mar 23, 2015 at 12:11:31AM +0100, Michel von Czettritz wrote:
>> Fixes checkpatch.pl error:
>> "ERROR: space required after that ',' (ctx:VxV)"
>>
>> Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
>> ---
>>  drivers/staging/sm750fb/sm750.c | 264 ++++++++++++++++++++--------------------
>>  1 file changed, 132 insertions(+), 132 deletions(-)
> 
> As I didn't take your first patch, the rest of the patches in this
> series do not apply.
> 
> Please fix them up, rebase on my latest staging-testing branch of
> staging.git, and resend so that I can apply them.
> 
> thanks,
> 
> greg k-h

I am, as a newbie, a bit confused. 
Should i resend my fixes with RESEND or with a v2 flag?
As Sudip i applied Lorenzos conflicting patches on statging-testing and rebased my patches on them. 
Should i wait until Lorenzos patches are applied?

regards Michel

> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* [PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval
From: Scot Doyle @ 2015-03-26 13:57 UTC (permalink / raw)
  To: Michael Kerrisk
  Cc: Greg Kroah-Hartman, Jiri Slaby, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.11.1503261341260.2164@local>

Add a Console Private CSI sequence to specify the current console's
cursor blink interval. The interval is specified as a number of
milliseconds until the next cursor display state toggle, from 50 to
65535.

Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
---
 man4/console_codes.4 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/man4/console_codes.4 b/man4/console_codes.4
index 34f7535..7d05076 100644
--- a/man4/console_codes.4
+++ b/man4/console_codes.4
@@ -377,6 +377,7 @@ ESC [ 15 ]      	T{
 Bring the previous console to the front
 (since Linux 2.6.0).
 T}
+ESC [ 16 ; \fIn\fP ]   	Set the cursor blink interval in milliseconds.
 .TE
 .SS Character sets
 The kernel knows about 4 translations of bytes into console-screen
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Scot Doyle @ 2015-03-26 13:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Tomi Valkeinen
  Cc: Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.11.1503261341260.2164@local>

vt now provides a cursor blink interval via vc_data. Use this
interval instead of the currently hardcoded 200 msecs. Store it in
fbcon_ops to avoid locking the console in cursor_timer_handler().

Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/video/console/fbcon.c | 10 +++++-----
 drivers/video/console/fbcon.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index b972106..05b1d1a 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -402,7 +402,7 @@ static void cursor_timer_handler(unsigned long dev_addr)
 	struct fbcon_ops *ops = info->fbcon_par;
 
 	queue_work(system_power_efficient_wq, &info->queue);
-	mod_timer(&ops->cursor_timer, jiffies + HZ/5);
+	mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
 }
 
 static void fbcon_add_cursor_timer(struct fb_info *info)
@@ -417,7 +417,7 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
 
 		init_timer(&ops->cursor_timer);
 		ops->cursor_timer.function = cursor_timer_handler;
-		ops->cursor_timer.expires = jiffies + HZ / 5;
+		ops->cursor_timer.expires = jiffies + ops->cur_blink_jiffies;
 		ops->cursor_timer.data = (unsigned long ) info;
 		add_timer(&ops->cursor_timer);
 		ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
@@ -1309,9 +1309,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 	if (fbcon_is_inactive(vc, info) || vc->vc_deccm != 1)
 		return;
 
-	if (vc->vc_cursor_type & 0x10)
-		fbcon_del_cursor_timer(info);
-	else
+	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
+	fbcon_del_cursor_timer(info);
+	if (!(vc->vc_cursor_type & 0x10))
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode = CM_ERASE) ? 0 : 1;
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..7aaa4ea 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -70,6 +70,7 @@ struct fbcon_ops {
 	struct fb_cursor cursor_state;
 	struct display *p;
         int    currcon;	                /* Current VC. */
+	int    cur_blink_jiffies;
 	int    cursor_flash;
 	int    cursor_reset;
 	int    blank_state;
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 1/3] vt: add cursor blink interval escape sequence
From: Scot Doyle @ 2015-03-26 13:54 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Michael Kerrisk, Jiri Slaby, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.11.1503261341260.2164@local>

Add an escape sequence to specify the current console's cursor blink
interval. The interval is specified as a number of milliseconds until
the next cursor display state toggle, from 50 to 65535. /proc/loadavg
did not show a difference with a one msec interval, but the lower
bound is set to 50 msecs since slower hardware wasn't tested.

Store the interval in the vc_data structure for later access by fbcon,
initializing the value to fbcon's current hardcoded value of 200 msecs.

Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/tty/vt/vt.c            | 9 +++++++++
 include/linux/console_struct.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 6e00572..ab1f173 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -135,6 +135,7 @@ const struct consw *conswitchp;
  */
 #define DEFAULT_BELL_PITCH	750
 #define DEFAULT_BELL_DURATION	(HZ/8)
+#define DEFAULT_CURSOR_BLINK_MS	200
 
 struct vc vc_cons [MAX_NR_CONSOLES];
 
@@ -1590,6 +1591,13 @@ static void setterm_command(struct vc_data *vc)
 		case 15: /* activate the previous console */
 			set_console(last_console);
 			break;
+		case 16: /* set cursor blink duration in msec */
+			if (vc->vc_npar >= 1 && vc->vc_par[1] >= 50 &&
+					vc->vc_par[1] <= USHRT_MAX)
+				vc->vc_cur_blink_ms = vc->vc_par[1];
+			else
+				vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
+			break;
 	}
 }
 
@@ -1717,6 +1725,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
 
 	vc->vc_bell_pitch = DEFAULT_BELL_PITCH;
 	vc->vc_bell_duration = DEFAULT_BELL_DURATION;
+	vc->vc_cur_blink_ms = DEFAULT_CURSOR_BLINK_MS;
 
 	gotoxy(vc, 0, 0);
 	save_cur(vc);
diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index e859c98..e329ee2 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -104,6 +104,7 @@ struct vc_data {
 	unsigned int    vc_resize_user;         /* resize request from user */
 	unsigned int	vc_bell_pitch;		/* Console bell pitch */
 	unsigned int	vc_bell_duration;	/* Console bell duration */
+	unsigned short	vc_cur_blink_ms;	/* Cursor blink duration */
 	struct vc_data **vc_display_fg;		/* [!] Ptr to var holding fg console for this display */
 	struct uni_pagedir *vc_uni_pagedir;
 	struct uni_pagedir **vc_uni_pagedir_loc; /* [!] Location of uni_pagedir variable for this console */
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 0/3] add cursor blink interval terminal escape sequence
From: Scot Doyle @ 2015-03-26 13:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michael Kerrisk
  Cc: Jiri Slaby, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150325111949.GA24230-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

v2: Add documentation to console_codes man page (man-pages repo)

This patch series adds an escape sequence to specify the current console's
cursor blink interval. The default interval is set to fbcon's currently 
hardcoded 200 msecs.

Scot Doyle (3):
  vt: add cursor blink interval escape sequence
  fbcon: use the cursor blink interval provided by vt
  console_codes.4: Add CSI sequence for cursor blink interval

 drivers/tty/vt/vt.c            |  9 +++++++++
 drivers/video/console/fbcon.c  | 10 +++++-----
 drivers/video/console/fbcon.h  |  1 +
 include/linux/console_struct.h |  1 +
 man4/console_codes.4           |  1 +
 5 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.1.0


^ permalink raw reply

* Re: [PATCH 02/13] staging: sm750: add missing spaces after ','
From: Greg KH @ 2015-03-26 12:06 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150322231131.GA17080@x230-arch.club.entropia.de>

On Mon, Mar 23, 2015 at 12:11:31AM +0100, Michel von Czettritz wrote:
> Fixes checkpatch.pl error:
> "ERROR: space required after that ',' (ctx:VxV)"
> 
> Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750.c | 264 ++++++++++++++++++++--------------------
>  1 file changed, 132 insertions(+), 132 deletions(-)

As I didn't take your first patch, the rest of the patches in this
series do not apply.

Please fix them up, rebase on my latest staging-testing branch of
staging.git, and resend so that I can apply them.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Juergen Gross @ 2015-03-26  4:38 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Luis R. Rodriguez
  Cc: luto, mingo, tglx, hpa, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied, linux-kernel, linux-fbdev, x86,
	xen-devel, Luis R. Rodriguez, Ingo Molnar, Daniel Vetter,
	Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Dave Hansen, Stefan Bader, ville.syrjala, david.vrabel,
	toshi.kani, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <20150325195941.GM25884@l.oracle.com>

On 03/25/2015 08:59 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 20, 2015 at 04:17:52PM -0700, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> It is possible to enable CONFIG_MTRR and up with it
>> disabled at run time and yet CONFIG_X86_PAT continues
>> to kick through fully functionally. This can happen
>
> s/fully/full/ ?
>
>
>> for instance on Xen where MTRR is not supported but
>> PAT is, this can happen now on Linux as of commit
>> 47591df50 by Juergen introduced as of v3.19.
>
> s/3.19/4.0/

No, 3.19 is correct.

Juergen

>>
>> Technically we should assume the proper CPU
>> bits would be set to disable MTRR but we can't
>> always rely on this. At least on the Xen Hypervisor
>> for instance only X86_FEATURE_MTRR was disabled
>> as of Xen 4.4 through Xen commit 586ab6a [0],
>> but not X86_FEATURE_K6_MTRR, X86_FEATURE_CENTAUR_MCR,
>> or X86_FEATURE_CYRIX_ARR for instance.
>
> Oh, could you send an patch for that to Xen please?
>>
>> x86 mtrr code relies on quite a bit of checks for
>> mtrr_if being set to check to see if MTRR did get
>> set up, instead of using that lets provide a generic
>> setter which when set we know MTRR is enabled. This
>
> s/we know MTRR is enabled/will let us know that MTRR is enabled/
>
>> also adds a few checks where they were not before
>> which could potentially safeguard ourselves against
>> incorrect usage of MTRR where this was not desirable.
>>
>> Where possible match error codes as if MTRR was
>> disabled on arch/x86/include/asm/mtrr.h.
>>
>> Lastly, since disabling MTRR can happen at run time
>> and we could end up with PAT enabled best record now
>> on our logs when MTRR is disabled.
>>
>> [0] ~/devel/xen (git::stable-4.5)$ git describe --contains 586ab6a
>> 4.4.0-rc1~18
>>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
>> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Antonino Daplas <adaplas@gmail.com>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: venkatesh.pallipadi@intel.com
>> Cc: Stefan Bader <stefan.bader@canonical.com>
>> Cc: konrad.wilk@oracle.com
>> Cc: ville.syrjala@linux.intel.com
>> Cc: david.vrabel@citrix.com
>> Cc: jbeulich@suse.com
>> Cc: toshi.kani@hp.com
>> Cc: bhelgaas@google.com
>> Cc: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: xen-devel@lists.xensource.com
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>> ---
>>   arch/x86/include/asm/mtrr.h        |  2 ++
>>   arch/x86/kernel/cpu/mtrr/cleanup.c |  2 +-
>>   arch/x86/kernel/cpu/mtrr/generic.c |  5 +++--
>>   arch/x86/kernel/cpu/mtrr/if.c      |  3 +++
>>   arch/x86/kernel/cpu/mtrr/main.c    | 31 ++++++++++++++++++++++---------
>>   5 files changed, 31 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
>> index f768f62..cade917 100644
>> --- a/arch/x86/include/asm/mtrr.h
>> +++ b/arch/x86/include/asm/mtrr.h
>> @@ -31,6 +31,7 @@
>>    * arch_phys_wc_add and arch_phys_wc_del.
>>    */
>>   # ifdef CONFIG_MTRR
>> +extern int mtrr_enabled;
>>   extern u8 mtrr_type_lookup(u64 addr, u64 end);
>>   extern void mtrr_save_fixed_ranges(void *);
>>   extern void mtrr_save_state(void);
>> @@ -50,6 +51,7 @@ extern int mtrr_trim_uncached_memory(unsigned long end_pfn);
>>   extern int amd_special_default_mtrr(void);
>>   extern int phys_wc_to_mtrr_index(int handle);
>>   #  else
>> +static const int mtrr_enabled;
>>   static inline u8 mtrr_type_lookup(u64 addr, u64 end)
>>   {
>>   	/*
>> diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
>> index 5f90b85..784dc55 100644
>> --- a/arch/x86/kernel/cpu/mtrr/cleanup.c
>> +++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
>> @@ -880,7 +880,7 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
>>   	 * Make sure we only trim uncachable memory on machines that
>>   	 * support the Intel MTRR architecture:
>>   	 */
>> -	if (!is_cpu(INTEL) || disable_mtrr_trim)
>> +	if (!is_cpu(INTEL) || disable_mtrr_trim || !mtrr_enabled)
>>   		return 0;
>>
>>   	rdmsr(MSR_MTRRdefType, def, dummy);
>> diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
>> index 09c82de..df321b2 100644
>> --- a/arch/x86/kernel/cpu/mtrr/generic.c
>> +++ b/arch/x86/kernel/cpu/mtrr/generic.c
>> @@ -116,7 +116,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat)
>>   	u8 prev_match, curr_match;
>>
>>   	*repeat = 0;
>> -	if (!mtrr_state_set)
>> +	/* generic_mtrr_ops is only set for generic_mtrr_ops */
>> +	if (!mtrr_state_set || !mtrr_enabled)
>>   		return 0xFF;
>>
>>   	if (!mtrr_state.enabled)
>> @@ -290,7 +291,7 @@ static void get_fixed_ranges(mtrr_type *frs)
>>
>>   void mtrr_save_fixed_ranges(void *info)
>>   {
>> -	if (cpu_has_mtrr)
>> +	if (mtrr_enabled && cpu_has_mtrr)
>>   		get_fixed_ranges(mtrr_state.fixed_ranges);
>>   }
>>
>> diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
>> index d76f13d..e9e001a 100644
>> --- a/arch/x86/kernel/cpu/mtrr/if.c
>> +++ b/arch/x86/kernel/cpu/mtrr/if.c
>> @@ -436,6 +436,9 @@ static int __init mtrr_if_init(void)
>>   {
>>   	struct cpuinfo_x86 *c = &boot_cpu_data;
>>
>> +	if (!mtrr_enabled)
>> +		return 0;
>> +
>>   	if ((!cpu_has(c, X86_FEATURE_MTRR)) &&
>>   	    (!cpu_has(c, X86_FEATURE_K6_MTRR)) &&
>>   	    (!cpu_has(c, X86_FEATURE_CYRIX_ARR)) &&
>> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
>> index ea5f363..7db9c47 100644
>> --- a/arch/x86/kernel/cpu/mtrr/main.c
>> +++ b/arch/x86/kernel/cpu/mtrr/main.c
>> @@ -59,6 +59,7 @@
>>   #define MTRR_TO_PHYS_WC_OFFSET 1000
>>
>>   u32 num_var_ranges;
>> +int mtrr_enabled;
>>
>>   unsigned int mtrr_usage_table[MTRR_MAX_VAR_RANGES];
>>   static DEFINE_MUTEX(mtrr_mutex);
>> @@ -84,6 +85,9 @@ static int have_wrcomb(void)
>>   {
>>   	struct pci_dev *dev;
>>
>> +	if (!mtrr_enabled)
>> +		return 0;
>> +
>>   	dev = pci_get_class(PCI_CLASS_BRIDGE_HOST << 8, NULL);
>>   	if (dev != NULL) {
>>   		/*
>> @@ -286,7 +290,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
>>   	int i, replace, error;
>>   	mtrr_type ltype;
>>
>> -	if (!mtrr_if)
>> +	if (!mtrr_enabled)
>>   		return -ENXIO;
>>
>>   	error = mtrr_if->validate_add_page(base, size, type);
>> @@ -388,6 +392,8 @@ int mtrr_add_page(unsigned long base, unsigned long size,
>>
>>   static int mtrr_check(unsigned long base, unsigned long size)
>>   {
>> +	if (!mtrr_enabled)
>> +		return -ENODEV;
>>   	if ((base & (PAGE_SIZE - 1)) || (size & (PAGE_SIZE - 1))) {
>>   		pr_warning("mtrr: size and base must be multiples of 4 kiB\n");
>>   		pr_debug("mtrr: size: 0x%lx  base: 0x%lx\n", size, base);
>> @@ -463,8 +469,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
>>   	unsigned long lbase, lsize;
>>   	int error = -EINVAL;
>>
>> -	if (!mtrr_if)
>> -		return -ENXIO;
>> +	if (!mtrr_enabled)
>> +		return -ENODEV;
>>
>>   	max = num_var_ranges;
>>   	/* No CPU hotplug when we change MTRR entries */
>> @@ -523,6 +529,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
>>    */
>>   int mtrr_del(int reg, unsigned long base, unsigned long size)
>>   {
>> +	if (!mtrr_enabled)
>> +		return -ENODEV;
>>   	if (mtrr_check(base, size))
>>   		return -EINVAL;
>>   	return mtrr_del_page(reg, base >> PAGE_SHIFT, size >> PAGE_SHIFT);
>> @@ -545,7 +553,7 @@ int arch_phys_wc_add(unsigned long base, unsigned long size)
>>   {
>>   	int ret;
>>
>> -	if (pat_enabled)
>> +	if (pat_enabled || !mtrr_enabled)
>>   		return 0;  /* Success!  (We don't need to do anything.) */
>>
>>   	ret = mtrr_add(base, size, MTRR_TYPE_WRCOMB, true);
>> @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
>>   	}
>>
>>   	if (mtrr_if) {
>> +		mtrr_enabled = true;
>>   		set_num_var_ranges();
>>   		init_table();
>>   		if (use_intel()) {
>> @@ -744,12 +753,13 @@ void __init mtrr_bp_init(void)
>>   				mtrr_if->set_all();
>>   			}
>>   		}
>> -	}
>> +	} else
>> +		pr_info("mtrr: system does not support MTRR\n");
>
>   'pr_warn' ?
>>   }
>>
>>   void mtrr_ap_init(void)
>>   {
>> -	if (!use_intel() || mtrr_aps_delayed_init)
>> +	if (!use_intel() || mtrr_aps_delayed_init || !mtrr_enabled)
>>   		return;
>>   	/*
>>   	 * Ideally we should hold mtrr_mutex here to avoid mtrr entries
>> @@ -774,6 +784,9 @@ void mtrr_save_state(void)
>>   {
>>   	int first_cpu;
>>
>> +	if (!mtrr_enabled)
>> +		return;
>> +
>>   	get_online_cpus();
>>   	first_cpu = cpumask_first(cpu_online_mask);
>>   	smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
>> @@ -782,7 +795,7 @@ void mtrr_save_state(void)
>>
>>   void set_mtrr_aps_delayed_init(void)
>>   {
>> -	if (!use_intel())
>> +	if (!use_intel() || !mtrr_enabled)
>>   		return;
>>
>>   	mtrr_aps_delayed_init = true;
>> @@ -810,7 +823,7 @@ void mtrr_aps_init(void)
>>
>>   void mtrr_bp_restore(void)
>>   {
>> -	if (!use_intel())
>> +	if (!use_intel() || !mtrr_enabled)
>>   		return;
>>
>>   	mtrr_if->set_all();
>> @@ -818,7 +831,7 @@ void mtrr_bp_restore(void)
>>
>>   static int __init mtrr_init_finialize(void)
>>   {
>> -	if (!mtrr_if)
>> +	if (!mtrr_enabled)
>>   		return 0;
>>
>>   	if (use_intel()) {
>> --
>> 2.3.2.209.gd67f9d5.dirty
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>


^ permalink raw reply

* Re: [PATCH v1 05/47] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-03-26  3:00 UTC (permalink / raw)
  To: Bjorn Helgaas, Arnd Bergmann, Linus Walleij, Stefano Stabellini,
	Julia Lawall, Peter Senna Tschudin
  Cc: Luis R. Rodriguez, Andy Lutomirski, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin, jgross, Jan Beulich, Borislav Petkov,
	Suresh Siddha, venkatesh.pallipadi, Dave Airlie,
	linux-kernel@vger.kernel.org, linux-fbdev, x86@kernel.org,
	xen-devel@lists.xenproject.org, Ingo Molnar, Daniel Vetter,
	Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Dave Hansen, Arnd Bergmann, Michael S. Tsirkin, Stefan Bader,
	Konrad Rzeszutek Wilk, Ville Syrjälä, David Vrabel,
	Toshi Kani, Roger Pau Monné, Benjamin Poirier, linux-pci
In-Reply-To: <CAErSpo5LfgTM879jmt58yLrXD6e7spsaX1wNDcc2ppU9JrMXOg@mail.gmail.com>

On Mon, Mar 23, 2015 at 12:20:47PM -0500, Bjorn Helgaas wrote:
> Hi Luis,
> 
> This seems OK to me, 

Great.

> but I'm curious about a few things.
> 
> On Fri, Mar 20, 2015 at 6:17 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > This allows drivers to take advantage of write-combining
> > when possible. Ideally we'd have pci_read_bases() just
> > peg an IORESOURCE_WC flag for us
> 
> We do set IORESOURCE_PREFETCH.  Do you mean something different?

I did not think we had a WC IORESOURCE flag. Are you saying that we can use
IORESOURCE_PREFETCH for that purpose? If so then great.  As I read a PCI BAR
can have PCI_BASE_ADDRESS_MEM_PREFETCH and when that's the case we peg
IORESOURCE_PREFETCH. That seems to be what I want indeed. Questions below.

> >  but where exactly
> > video devices memory lie varies *largely* and at times things
> > are mixed with MMIO registers, sometimes we can address
> > the changes in drivers, other times the change requires
> > intrusive changes.
> 
> What does a video device address have to do with this?  I do see that
> if a BAR maps only a frame buffer, the device might be able to mark it
> prefetchable, while if the BAR mapped both a frame buffer and some
> registers, it might not be able to make it prefetchable.  But that
> doesn't seem like it depends on the *address*.

I meant the offsets for each of those, either registers or framebuffer,
and that typically they are mixed (primarily on older devices), so indeed your
summary of the problem is what I meant. Let's remember that we are trying to
take advantage of PAT here when available and avoid MTRR in that case, do we
know that the same PCI BARs that have always historically used MTRRs had
IORESOURCE_PREFETCH set, is that a fair assumption ? I realize they are
different things -- but its precisely why I ask.

> pci_iomap_range() already makes a cacheable mapping if
> IORESOURCE_CACHEABLE; I'm guessing that you would like it to
> automatically use WC if the BAR if IORESOURCE_PREFETCH, e.g.,
> 
>   if (flags & IORESOURCE_CACHEABLE)
>     return ioremap(start, len);
>   if (flags & IORESOURCE_PREFETCH)
>     return ioremap_wc(start, len);
>   return ioremap_nocache(start, len);

Indeed, that's exactly what I think we should strive towards.

> Is there a reason not to do that?

This depends on the exact defintion of IORESOURCE_PREFETCH and
PCI_BASE_ADDRESS_MEM_PREFETCH and how they are used all over and
accross *all devices*. This didn't look promising for starters:

include/uapi/linux/pci_regs.h:#define  PCI_BASE_ADDRESS_MEM_PREFETCH    0x08    /* prefetchable? */

PCI_BASE_ADDRESS_MEM_PREFETCH seems to be BAR specific, so a few questions:

1) Can we rest assured for instance that if we check for
PCI_BASE_ADDRESS_MEM_PREFETCH and if set that it will *only* be set on a full
PCI BAR if the full PCI BAR does want WC? If not this can regress
functionality. That seems risky. It however would not be risky if we used
another API that did look for IORESOURCE_PREFETCH and if so use ioremap_wc() --
that way only drivers we know that do use the full PCI bar would use this API.
There's a bit of a problem with this though:

2) Do we know that if a *full PCI BAR* is used for WC that
PCI_BASE_ADDRESS_MEM_PREFETCH *was* definitely set for the PCI BAR? If so then
the API usage would be restricted only to devices that we know *do* adhere to
this. That reduces the possible uses for older drivers and can create
regressions if used loosely without verification... but..

3) If from now on we get folks to commit to uset PCI_BASE_ADDRESS_MEM_PREFETCH
for full PCI BARs that do want WC perhaps newer devices / drivers will use
this very consistently ? Can we bank on that and is it worth it ?

4) If a PCI BAR *does not* have PCI_BASE_ADDRESS_MEM_PREFETCH do we know it
must not never want WC ?

If we don't have certainty on any of the above I'm afraid we can't do much
right now but perhaps we can push towards better use of PCI_BASE_ADDRESS_MEM_PREFETCH
and hope folks will only use this for the full PCI BAR only if WC is desired.

Thoughts?

> > Although there is also arch_phys_wc_add() that makes use of
> > architecture specific write-combinging alternatives (MTRR on
> > x86 when a system does not have PAT) we void polluting
> > pci_iomap() space with it and force drivers and subsystems
> > that want to use it to be explicit.
> >
> > There are a few motivations for this:
> >
> > a) Take advantage of PAT when available
> >
> > b) Help bury MTRR code away, MTRR is architecture specific and on
> >    x86 its replaced by PAT
> >
> > c) Help with the goal of eventually using _PAGE_CACHE_UC over
> >    _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (de33c442e)
> > ...
> 
> > +void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
> > +                                int bar,
> > +                                unsigned long offset,
> > +                                unsigned long maxlen)
> > +{
> > +       resource_size_t start = pci_resource_start(dev, bar);
> > +       resource_size_t len = pci_resource_len(dev, bar);
> > +       unsigned long flags = pci_resource_flags(dev, bar);
> > +
> > +       if (len <= offset || !start)
> > +               return NULL;
> > +       len -= offset;
> > +       start += offset;
> > +       if (maxlen && len > maxlen)
> > +               len = maxlen;
> > +       if (flags & IORESOURCE_IO)
> > +               return __pci_ioport_map(dev, start, len);
> > +       if (flags & IORESOURCE_MEM)
> 
> Should we log a note in dmesg if the BAR is *not* IORESOURCE_PREFETCH?
>  I know the driver might know it's safe even if the device didn't mark
> the BAR as prefetchable, but it does seem like an easy way for a
> driver to shoot itself in the foot.

You tell me. I would fear this may not be consistent and we'd end up
having bug reports open for something that has historically been a
non-issue. The above questions can help us gauge the risk of this.

  Luis

^ permalink raw reply

* Re: [PATCHv5 04/11] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Thomas Niederprüm @ 2015-03-25 22:14 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt, linux-fbdev, linux-kernel
In-Reply-To: <5512D91B.9090709@schinagl.nl>

Am Wed, 25 Mar 2015 16:49:47 +0100
schrieb Olliver Schinagl <oliver@schinagl.nl>:

> Hey Thomas,
> 
> awesome the time you put into this driver. It does what I was working
> on the last few weeks, but with a much bigger bang ;)
> 
> I personally have a ssd1309 and will be submitting the patches for
> that after testing your patch-set.

Yeah, it turned out to be quite some work. But I'm glad to see that it
might turn out useful for you and I'm looking forward to see support
added for even more controllers.

> On 24-03-15 22:23, Thomas Niederprüm wrote:
> > The 130X controllers are very similar from the configuration point
> > of view. The configuration registers for the SSD1305/6/7 are bit
> > identical (except the the VHCOM register and the the default values
> > for clock setup register). This patch unifies the init code of the
> > controller and adds hardware specific properties to DT that are
> > needed to correctly initialize the device.
> >
> > The SSD130X can be wired to the OLED panel in various ways. Even
> > for the same controller this wiring can differ from one display
> > module to another and can not be probed by software. The added DT
> > properties reflect these hardware decisions of the display module
> > manufacturer. The 'com-sequential', 'com-lrremap' and 'com-invdir'
> > values define different possibilities for the COM signals pin
> > configuration and readout direction of the video memory. The
> > 'segment-no-remap' allows the inversion of the memory-to-pin
> > mapping ultimately inverting the order of the controllers output
> > pins. The 'prechargepX' values need to be adapted according to the
> > capacitance of the OLEDs pixel cells.
> >
> > So far these hardware specific bits are hard coded in the init
> > code, making the driver usable only for one certain wiring of the
> > controller. This patch makes the driver usable with all possible
> > hardware setups, given a valid hw description in DT. If these
> > values are not set in DT the default values, as they are set in the
> > ssd1307 init code right now, are used. This implies that without
> > the corresponding DT property "segment-no-remap" the segment remap
> > of the ssd130X controller gets activated. Even though this is not
> > the default behaviour according to the datasheet it maintains
> > backward compatibility with older DTBs.
> >
> > Note that the SSD1306 does not seem to be using the configuration
> > written to the registers at all. Therefore this patch does not try
> > to maintain these values without changes in DT. For reference an
> > example is added to the DT bindings documentation that reproduces
> > the configuration that is set in the current init code.
> >
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> >   .../devicetree/bindings/video/ssd1307fb.txt        |  21 +++
> >   drivers/video/fbdev/ssd1307fb.c                    | 192
> > ++++++++++++--------- 2 files changed, 134 insertions(+), 79
> > deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > 7a12542..637690f 100644 ---
> > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
> > +15,16 @@ Required properties:
> >
> >   Optional properties:
> >     - reset-active-low: Is the reset gpio is active on physical low?
> > +  - solomon,segment-no-remap: Display needs normal (non-inverted)
> > data column
> > +                              to segment mapping
> > +  - solomon,com-sequential: Display uses sequential COM pin
> > configuration
> > +  - solomon,com-lrremap: Display uses left-right COM pin remap
> > +  - solomon,com-invdir: Display uses inverted COM pin scan
> > direction
> > +  - solomon,com-offset: Number of the COM pin wired to the first
> > display line
> > +  - solomon,prechargep1: Length of deselect period (phase 1) in
> > clock cycles.
> > +  - solomon,prechargep2: Length of precharge period (phase 2) in
> > clock cycles.
> > +                         This needs to be the higher, the higher
> > the capacitance
> > +                         of the OLED's pixels is
> Aren't the height, width and page-offset also optional? They should
> be set to sane defaults when not supplied. Though the default (which
> is the max really) width/height depends on the actual attached
> display to the chip (I mention this in the vcomh section too below,
> the controller's width/height would be the absolute maximum values,
> the display can always be smaller).
> 
> Same goes to say for the precharge, and probably other parameters. I 
> don't think we have a proper framework to handle the attached
> displays to a controller chip at the moment anyhow?
> 
> >
> >   [0]: Documentation/devicetree/bindings/pwm/pwm.txt
> >
> <snip>
> > +
> > +	/* Set COM direction */
> > +	com_invdir = 0xc0 | (par->com_invdir & 0xf) << 3;
> Shouldn't this be par->com_invdir & 0x1 ?
> The datasheet only mentions 1 bit change for the ssd1306, ssd1307 and 
> ssd1309. I dont' know what the ssd1305 does here though.

Yes, you are right, it should be  par->com_invdir &0x1. The
register of the 1305 is identical to the 1306/7.

> 
> > +	ret = ssd1307fb_write_cmd(par->client,  com_invdir);
> >   	if (ret < 0)
> >   		return ret;
> <snip>
> >
> > -	ret = ssd1307fb_write_cmd(par->client, 0x14);
> > -	if (ret < 0)
> > -		return ret;
> > +		ret = ssd1307fb_write_cmd(par->client, 0x14);
> > +		if (ret < 0)
> > +			return ret;
> > +	};
> I had to look hard for this setting, as my old datasheet had omitted
> the charge pump def.
> 
> but wouldn't you want something like (0x10 | 
> par->device_info->need_chargepump & 0x1 << 2) & 0x14 to follow your 
> previous styles?
> 
> Only bit 2 determines the state of the charge-pump. And I guess it 
> should be safe to use on the other chips as well. My 1309 doesn't
> seem to have a problem with it, I'm working a lot on this display so
> it will be tested thoroughly in the next few weeks.

I didn't look into this so far, I just took that command from the
ssd1306 init code. I just checked the datasheet of the 1306 and I agree
that par->device_info->need_chargepump & 0x1 << 2) & 0x14 should be
what we want here, assuming that the other controllers make no use of
the register 0xBD. I can confirm that my 1305 based display module
seems not to use it. I'll include your suggestion for the next round.

> >
> >   	/* Switch to horizontal addressing mode */
> >   	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_ADDRESS_MODE); @@ -393,6 +390,7 @@ static int
> > ssd1307fb_ssd1306_init(struct ssd1307fb_par *par) if (ret < 0)
> >   		return ret;
> >
> <snip>
> >
> > -static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
> > -	.init	= ssd1307fb_ssd1306_init,
> > +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
> > +	.default_vcomh = 0x20,
> > +	.default_dclk_div = 0,
> > +	.default_dclk_frq = 8,
> > +	.need_pwm = 0,
> > +	.need_chargepump = 1,
> > +};
> This device info struct, is this really always fixed data? We use a 
> ssd1309 with a MI12864MO i think and the combination of these two
> parts defines atleast the vcomh I would imagine? There are only a
> very limited number of consumers of this driver so for now it won't
> make much of a difference, just mentioning it as it is a configurable
> so it may matter depending on the physical display.

Note that the default_vcomh just holds the default value for vcomh as
stated in the controller's datasheet. It becomes necessary to define
those on a per controller basis since this one of the very few
registers that actually differ between the different ssd130X. As this
is only the default value this is fixed data.
Once it becomes relevant to set this according to the hardware of the
display module (controller + panel) I guess one has to introduce a DT
property for this. For my particular device (see below) the default
value was working nicely, which is why I did not dive into this.

> > +
> > +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1307_deviceinfo = {
> > +	.default_vcomh = 0x20,
> > +	.default_dclk_div = 1,
> > +	.default_dclk_frq = 12,
> > +	.need_pwm = 1,
> > +	.need_chargepump = 0,
> >   };
> >
> >   static const struct of_device_id ssd1307fb_of_match[] = {
> >   	{
> >   		.compatible = "solomon,ssd1306fb-i2c",
> > -		.data = (void *)&ssd1307fb_ssd1306_ops,
> > +		.data = (void *)&ssd1307fb_ssd1306_deviceinfo,
> >   	},
> >   	{
> >   		.compatible = "solomon,ssd1307fb-i2c",
> > -		.data = (void *)&ssd1307fb_ssd1307_ops,
> > +		.data = (void *)&ssd1307fb_ssd1307_deviceinfo,
> >   	},
> >   	{},
> >   };
> > @@ -468,8 +479,8 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, par->info = info;
> >   	par->client = client;
> >
> > -	par->ops = (struct ssd1307fb_ops
> > *)of_match_device(ssd1307fb_of_match,
> > -
> > &client->dev)->data;
> > +	par->device_info = (struct ssd1307fb_deviceinfo
> > *)of_match_device(
> > +			ssd1307fb_of_match, &client->dev)->data;
> >
> >   	par->reset = of_get_named_gpio(client->dev.of_node,
> >   					 "reset-gpios", 0);
> > @@ -487,6 +498,27 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, if (of_property_read_u32(node, "solomon,page-offset",
> > &par->page_offset)) par->page_offset = 1;
> While your re-doing the whole driver, why use a default page_offset
> of 1? 0 makes far more sense (its the default on my board also) and 
> logically, we'd start counting at 0 as programmers ;) Also, the 
> datasheet uses 0 as a default preset.

the page_offset is not a quantity that appears in the datasheet. Don't
mix it up with the com-offset (that is referred to as offset in the
datsheet). the page_offset is used in the init code to set the upper
limit of the page address range. 

> >
> > +	if (of_property_read_u32(node, "solomon,com-offset",
> > &par->com_offset))
> > +		par->com_offset = 0;
> > +
> > +	if (of_property_read_u32(node, "solomon,prechargep1",
> > &par->prechargep1))
> > +		par->prechargep1 = 2;
> > +
> > +	if (of_property_read_u32(node, "solomon,prechargep2",
> > &par->prechargep2))
> > +		par->prechargep2 = 0;
> Why set the default to 0 here? The datasheet sets it to 0x2 by
> default for example.

You are right, I will correct this.

> > +
> > +	par->seg_remap = !of_property_read_bool(node,
> > "solomon,segment-no-remap");
> > +	par->com_seq = of_property_read_bool(node,
> > "solomon,com-sequential");
> all the other parameters are somewhat abbreviated, or the property
> name matches the variable name. I'd just abbreviate it to com-seq
> clear and short.

Ok, why not. I will abbreviate the name for v6

> 
> 
> Overal, the driver works and can have a Tested-by: Olliver Schinagl 
> <o.schinagl@ultimaker.com> from me (for the 1309 which I'll submit
> very soon.

Great!

> 
> Olliver
> 
> P.S. Do you have a link for your display? I'm curious as to how it
> looks.

I'm using those two (very similar) displays:
http://www.newhavendisplay.com/nhd22312832ucy3-p-3787.html
http://www.newhavendisplay.com/nhd22312832umb3-p-6086.html

Thomas

^ permalink raw reply

* Re: [PATCHv5 11/11] fbdev: ssd1307fb: Add blank mode
From: Maxime Ripard @ 2015-03-25 22:00 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Thomas Niederprüm, plagnioj, tomi.valkeinen, kernel,
	shawn.guo, robh+dt, linux-fbdev, linux-kernel
In-Reply-To: <5512D92D.3020809@schinagl.nl>

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

On Wed, Mar 25, 2015 at 04:50:05PM +0100, Olliver Schinagl wrote:
> Hey Thomas,
> 
> On 24-03-15 22:23, Thomas Niederprüm wrote:
> >This patch adds ssd1307fb_blank() to make the framebuffer capable
> >of blanking.
> >
> >Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >---
> >  drivers/video/fbdev/ssd1307fb.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> >diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> >index 061cc95..9101b27 100644
> >--- a/drivers/video/fbdev/ssd1307fb.c
> >+++ b/drivers/video/fbdev/ssd1307fb.c
> >@@ -238,6 +238,18 @@ static ssize_t ssd1307fb_write(struct fb_info *info, const char __user *buf,
> >  	return count;
> >  }
> >
> >+static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
> >+{
> >+	struct ssd1307fb_par *par = info->par;
> >+	int ret;
> >+
> >+	if (blank_mode != FB_BLANK_UNBLANK)
> >+		ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_OFF);
> >+	else
> >+		ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
> I'd probably add an extra return, or drop the ret var at all and just return
> the function.
> 
> or even shorter :)
> return sd1307fb_write_cmd(par->client, (blank_mode != FB_BLANK_UNBLANK) ?
> SSD1307FB_DISPLAY_OFF : SSD1307FB_DISPLAY_ON;

In some domains, shorter is not better ;)

This is one of these cases. It's much clearer as it is.

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [Xen-devel] [PATCH v1 04/47] pci: add pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-03-25 20:39 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Arjan van de Ven, Arjan van de Ven
  Cc: Luis R. Rodriguez, luto, mingo, tglx, hpa, jgross, JBeulich, bp,
	suresh.b.siddha, venkatesh.pallipadi, airlied, linux-fbdev,
	Antonino Daplas, Daniel Vetter, x86, linux-kernel, Tomi Valkeinen,
	xen-devel, Ingo Molnar, Jean-Christophe Plagniol-Villard
In-Reply-To: <20150325200346.GO25884@l.oracle.com>

On Wed, Mar 25, 2015 at 04:03:46PM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 20, 2015 at 04:17:54PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > This lets drivers take advanate of PAT when available. This
> 
> s/advanate/advantage/

Amended.

> > should help with the transition of converting video drivers over
> > to ioremap_wc() to help with the goal of eventually using
> > _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
> > ioremap_nocache() (de33c442e)
> 
> Please mention the title of the patch too:
> 
> "x86 PAT: fix performance drop for glx, use UC minus for ioremap(), ioremap_nocache() and pci_mmap_page_range()"

Added.

> > 
> > Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > ---
> >  drivers/pci/pci.c   | 14 ++++++++++++++
> >  include/linux/pci.h |  1 +
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 81f06e8..6afd507 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -137,6 +137,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
> >  				     pci_resource_len(pdev, bar));
> >  }
> >  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
> > +
> > +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
> > +{
> > +	/*
> > +	 * Make sure the BAR is actually a memory resource, not an IO resource
> > +	 */
> > +	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> > +		WARN_ON(1);
> 
> Would it be better to use dev_warn ? That way you can see which BDF it is?
> 
> Thought WARN will give a nice stack-trace that should easily point to the
> driver so perhaps not.. Either way - up to you.

I'm sticking to the style and use as with pci_ioremap_bar(). Whatever we pick
we should make both use the same. More information is always better and
since we do have dev_warn(), it would be nice to use that however within
its use on both pci_ioremap_wc_bar() and pci_ioremap_bar() we have
a use of the pdev with pci_resource_flags() and I believe if pdev is NULL
we'd get a NULL dereference (dev_driver_string() is used), so it would
seem it might be best to stick with a simple WARN_ON(). Arjan, any
preference? Obviously if pdev is NULL your driver is dumb but as folks
develop drivers this should be expected.

 Luis

^ permalink raw reply

* Re: [PATCHv5 11/11] fbdev: ssd1307fb: Add blank mode
From: Thomas Niederprüm @ 2015-03-25 20:33 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: plagnioj, tomi.valkeinen, maxime.ripard, kernel, shawn.guo,
	robh+dt, linux-fbdev, linux-kernel
In-Reply-To: <5512D92D.3020809@schinagl.nl>

Am Wed, 25 Mar 2015 16:50:05 +0100
schrieb Olliver Schinagl <oliver@schinagl.nl>:

> Hey Thomas,
> 
> On 24-03-15 22:23, Thomas Niederprüm wrote:
> > This patch adds ssd1307fb_blank() to make the framebuffer capable
> > of blanking.
> >
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> >   drivers/video/fbdev/ssd1307fb.c | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 061cc95..9101b27 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -238,6 +238,18 @@ static ssize_t ssd1307fb_write(struct fb_info
> > *info, const char __user *buf, return count;
> >   }
> >
> > +static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
> > +{
> > +	struct ssd1307fb_par *par = info->par;
> > +	int ret;
> > +
> > +	if (blank_mode != FB_BLANK_UNBLANK)
> > +		ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_OFF);
> > +	else
> > +		ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_ON);
> I'd probably add an extra return, or drop the ret var at all and just 
> return the function.
> 
> or even shorter :)
> return sd1307fb_write_cmd(par->client, (blank_mode !> FB_BLANK_UNBLANK) ? SSD1307FB_DISPLAY_OFF : SSD1307FB_DISPLAY_ON;

Wow, short and elegant. Thanks for the hint, I will include it in v6.

Thomas



^ permalink raw reply

* Re: [PATCHv5 09/11] fbdev: ssd1307fb: Add module parameter to set the initial contrast
From: Thomas Niederprüm @ 2015-03-25 20:10 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: plagnioj, tomi.valkeinen, kernel, shawn.guo, robh+dt, linux-fbdev,
	linux-kernel
In-Reply-To: <20150324221636.GE23664@lukather>

Am Tue, 24 Mar 2015 15:16:36 -0700
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> On Tue, Mar 24, 2015 at 10:23:56PM +0100, Thomas Niederprüm wrote:
> > This patch adds the module parameter "contrast" to determine the
> > contrast value that is used to initialize the display. This
> > setting applies to all instances of the driver.
> > 
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> 
> Do we still need this patch given that the next one is doing the exact
> same thing?
I was kind of expecting this objection. I added this module parameter
since it suits my use case where I always run the display at the lowest
possible contrast. With this module parameter I can specify this need
at module load time and I do not need udev or to touch the backlight
controls manually every time the driver is loaded. I see that this is
probably a (too) weak argument, so if nobody complains I will remove
this patch for the next round.

Thomas

^ permalink raw reply

* Re: [PATCH v1 05/47] pci: add pci_iomap_wc() variants
From: Konrad Rzeszutek Wilk @ 2015-03-25 20:07 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied, linux-kernel, linux-fbdev, x86,
	xen-devel, Luis R. Rodriguez, Ingo Molnar, Daniel Vetter,
	Bjorn Helgaas, Antonino Daplas, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Dave Hansen, Arnd Bergmann, Michael S. Tsirkin,
	Stefan Bader, ville.syrjala, david.vrabel, toshi.kani,
	Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-6-git-send-email-mcgrof@do-not-panic.com>

On Fri, Mar 20, 2015 at 04:17:55PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> This allows drivers to take advantage of write-combining
> when possible. Ideally we'd have pci_read_bases() just
> peg an IORESOURCE_WC flag for us but where exactly
> video devices memory lie varies *largely* and at times things
> are mixed with MMIO registers, sometimes we can address
> the changes in drivers, other times the change requires
> intrusive changes.
> 
> Although there is also arch_phys_wc_add() that makes use of
> architecture specific write-combinging alternatives (MTRR on

combinging?

> x86 when a system does not have PAT) we void polluting
> pci_iomap() space with it and force drivers and subsystems
> that want to use it to be explicit.
> 
> There are a few motivations for this:
> 
> a) Take advantage of PAT when available
> 
> b) Help bury MTRR code away, MTRR is architecture specific and on
>    x86 its replaced by PAT
> 
> c) Help with the goal of eventually using _PAGE_CACHE_UC over
>    _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (de33c442e)
> 
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: venkatesh.pallipadi@intel.com
> Cc: Stefan Bader <stefan.bader@canonical.com>
> Cc: konrad.wilk@oracle.com
> Cc: ville.syrjala@linux.intel.com
> Cc: david.vrabel@citrix.com
> Cc: jbeulich@suse.com
> Cc: toshi.kani@hp.com
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: xen-devel@lists.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  include/asm-generic/pci_iomap.h | 14 ++++++++++
>  lib/pci_iomap.c                 | 61 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
> index 7389c87..b1e17fc 100644
> --- a/include/asm-generic/pci_iomap.h
> +++ b/include/asm-generic/pci_iomap.h
> @@ -15,9 +15,13 @@ struct pci_dev;
>  #ifdef CONFIG_PCI
>  /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
>  extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
> +extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max);
>  extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
>  				     unsigned long offset,
>  				     unsigned long maxlen);
> +extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
> +					unsigned long offset,
> +					unsigned long maxlen);
>  /* Create a virtual mapping cookie for a port on a given PCI device.
>   * Do not call this directly, it exists to make it easier for architectures
>   * to override */
> @@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
>  	return NULL;
>  }
>  
> +static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max)
> +{
> +	return NULL;
> +}
>  static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
>  					    unsigned long offset,
>  					    unsigned long maxlen)
>  {
>  	return NULL;
>  }
> +static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
> +					       unsigned long offset,
> +					       unsigned long maxlen)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  #endif /* __ASM_GENERIC_IO_H */
> diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
> index bcce5f1..30b65ae 100644
> --- a/lib/pci_iomap.c
> +++ b/lib/pci_iomap.c
> @@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev,
>  EXPORT_SYMBOL(pci_iomap_range);
>  
>  /**
> + * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR number
> + * @offset: map memory at the given offset in BAR
> + * @maxlen: max length of the memory to map
> + *
> + * Using this function you will get a __iomem address to your device BAR.
> + * You can access it using ioread*() and iowrite*(). These functions hide
> + * the details if this is a MMIO or PIO address space and will just do what
> + * you expect from them in the correct way. When possible write combining
> + * is used.
> + *
> + * @maxlen specifies the maximum length to map. If you want to get access to
> + * the complete BAR from offset to the end, pass %0 here.

s/%0/0 ? Or is that some special syntax?

> + * */
> +void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
> +				 int bar,
> +				 unsigned long offset,
> +				 unsigned long maxlen)
> +{
> +	resource_size_t start = pci_resource_start(dev, bar);
> +	resource_size_t len = pci_resource_len(dev, bar);
> +	unsigned long flags = pci_resource_flags(dev, bar);
> +
> +	if (len <= offset || !start)
> +		return NULL;
> +	len -= offset;
> +	start += offset;
> +	if (maxlen && len > maxlen)
> +		len = maxlen;
> +	if (flags & IORESOURCE_IO)
> +		return __pci_ioport_map(dev, start, len);
> +	if (flags & IORESOURCE_MEM)
> +		return ioremap_wc(start, len);
> +	/* What? */
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
> +
> +/**
>   * pci_iomap - create a virtual mapping cookie for a PCI BAR
>   * @dev: PCI device that owns the BAR
>   * @bar: BAR number
> @@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
>  	return pci_iomap_range(dev, bar, 0, maxlen);
>  }
>  EXPORT_SYMBOL(pci_iomap);
> +
> +/**
> + * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR number
> + * @maxlen: length of the memory to map
> + *
> + * Using this function you will get a __iomem address to your device BAR.
> + * You can access it using ioread*() and iowrite*(). These functions hide
> + * the details if this is a MMIO or PIO address space and will just do what
> + * you expect from them in the correct way. When possible write combining
> + * is used.
> + *
> + * @maxlen specifies the maximum length to map. If you want to get access to
> + * the complete BAR without checking for its length first, pass %0 here.
> + * */
> +void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
> +{
> +	return pci_iomap_wc_range(dev, bar, 0, maxlen);
> +}
> +EXPORT_SYMBOL_GPL(pci_iomap_wc);
>  #endif /* CONFIG_PCI */
> -- 
> 2.3.2.209.gd67f9d5.dirty
> 

^ permalink raw reply

* Re: [PATCH v1 04/47] pci: add pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-03-25 20:06 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Juergen Gross, Jan Beulich, Borislav Petkov, Suresh Siddha,
	venkatesh.pallipadi, Dave Airlie, linux-kernel@vger.kernel.org,
	Linux Fbdev development list, X86 ML,
	xen-devel@lists.xenproject.org, Ingo Molnar, Daniel Vetter,
	Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <CALCETrXkZipqdY_faY+JOyuqcQKerk_CKH90ChDLTgCJPMKLug@mail.gmail.com>

On Fri, Mar 20, 2015 at 04:50:32PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > This lets drivers take advanate of PAT when available. This
> > should help with the transition of converting video drivers over
> > to ioremap_wc() to help with the goal of eventually using
> > _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
> > ioremap_nocache() (de33c442e)
> >
> > Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > ---
> >  drivers/pci/pci.c   | 14 ++++++++++++++
> >  include/linux/pci.h |  1 +
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 81f06e8..6afd507 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -137,6 +137,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
> >                                      pci_resource_len(pdev, bar));
> >  }
> >  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
> > +
> > +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
> > +{
> > +       /*
> > +        * Make sure the BAR is actually a memory resource, not an IO resource
> > +        */
> > +       if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> > +               WARN_ON(1);
> > +               return NULL;
> > +       }
> 
> if (WARN_ON(...))?

Sure, they are equivalent however this follows the same exact style as
pci_ioremap_bar() so if we change this one might as well change the style of
pci_ioremap_bar() as well. Let me know if there is any preference. I personally
don't mind the extra line as it shortens the check.

 Luis

^ permalink raw reply

* Re: [Xen-devel] [PATCH v1 04/47] pci: add pci_ioremap_wc_bar()
From: Konrad Rzeszutek Wilk @ 2015-03-25 20:03 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied, linux-fbdev, Antonino Daplas,
	Daniel Vetter, Luis R. Rodriguez, x86, linux-kernel,
	Tomi Valkeinen, xen-devel, Ingo Molnar,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <1426893517-2511-5-git-send-email-mcgrof@do-not-panic.com>

On Fri, Mar 20, 2015 at 04:17:54PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> This lets drivers take advanate of PAT when available. This

s/advanate/advantage/
> should help with the transition of converting video drivers over
> to ioremap_wc() to help with the goal of eventually using
> _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
> ioremap_nocache() (de33c442e)

Please mention the title of the patch too:

"x86 PAT: fix performance drop for glx, use UC minus for ioremap(), ioremap_nocache() and pci_mmap_page_range()"
> 
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  drivers/pci/pci.c   | 14 ++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 81f06e8..6afd507 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -137,6 +137,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>  				     pci_resource_len(pdev, bar));
>  }
>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
> +
> +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
> +{
> +	/*
> +	 * Make sure the BAR is actually a memory resource, not an IO resource
> +	 */
> +	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> +		WARN_ON(1);

Would it be better to use dev_warn ? That way you can see which BDF it is?

Thought WARN will give a nice stack-trace that should easily point to the
driver so perhaps not.. Either way - up to you.

> +		return NULL;
> +	}
> +	return ioremap_wc(pci_resource_start(pdev, bar),
> +			  pci_resource_len(pdev, bar));
> +}
> +EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
>  #endif
>  
>  #define PCI_FIND_CAP_TTL	48
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 211e9da..c235b09 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1667,6 +1667,7 @@ static inline void pci_mmcfg_late_init(void) { }
>  int pci_ext_cfg_avail(void);
>  
>  void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar);
> +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar);
>  
>  #ifdef CONFIG_PCI_IOV
>  int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
> -- 
> 2.3.2.209.gd67f9d5.dirty
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply

* Re: [PATCHv5 04/11] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Thomas Niederprüm @ 2015-03-25 20:03 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: plagnioj, tomi.valkeinen, kernel, shawn.guo, robh+dt, linux-fbdev,
	linux-kernel
In-Reply-To: <20150324221428.GD23664@lukather>

Am Tue, 24 Mar 2015 15:14:28 -0700
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> On Tue, Mar 24, 2015 at 10:23:51PM +0100, Thomas Niederprüm wrote:
> > The 130X controllers are very similar from the configuration point
> > of view. The configuration registers for the SSD1305/6/7 are bit
> > identical (except the the VHCOM register and the the default values
> > for clock setup register). This patch unifies the init code of the
> > controller and adds hardware specific properties to DT that are
> > needed to correctly initialize the device.
> > 
> > The SSD130X can be wired to the OLED panel in various ways. Even
> > for the same controller this wiring can differ from one display
> > module to another and can not be probed by software. The added DT
> > properties reflect these hardware decisions of the display module
> > manufacturer. The 'com-sequential', 'com-lrremap' and 'com-invdir'
> > values define different possibilities for the COM signals pin
> > configuration and readout direction of the video memory. The
> > 'segment-no-remap' allows the inversion of the memory-to-pin
> > mapping ultimately inverting the order of the controllers output
> > pins. The 'prechargepX' values need to be adapted according to the
> > capacitance of the OLEDs pixel cells.
> > 
> > So far these hardware specific bits are hard coded in the init
> > code, making the driver usable only for one certain wiring of the
> > controller. This patch makes the driver usable with all possible
> > hardware setups, given a valid hw description in DT. If these
> > values are not set in DT the default values, as they are set in the
> > ssd1307 init code right now, are used. This implies that without
> > the corresponding DT property "segment-no-remap" the segment remap
> > of the ssd130X controller gets activated. Even though this is not
> > the default behaviour according to the datasheet it maintains
> > backward compatibility with older DTBs.
> > 
> > Note that the SSD1306 does not seem to be using the configuration
> > written to the registers at all. Therefore this patch does not try
> > to maintain these values without changes in DT. For reference an
> > example is added to the DT bindings documentation that reproduces
> > the configuration that is set in the current init code.
> > 
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> >  .../devicetree/bindings/video/ssd1307fb.txt        |  21 +++
> >  drivers/video/fbdev/ssd1307fb.c                    | 192
> > ++++++++++++--------- 2 files changed, 134 insertions(+), 79
> > deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > 7a12542..637690f 100644 ---
> > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@ -15,6
> > +15,16 @@ Required properties: 
> >  Optional properties:
> >    - reset-active-low: Is the reset gpio is active on physical low?
> > +  - solomon,segment-no-remap: Display needs normal (non-inverted)
> > data column
> > +                              to segment mapping
> > +  - solomon,com-sequential: Display uses sequential COM pin
> > configuration
> > +  - solomon,com-lrremap: Display uses left-right COM pin remap
> > +  - solomon,com-invdir: Display uses inverted COM pin scan
> > direction
> > +  - solomon,com-offset: Number of the COM pin wired to the first
> > display line
> > +  - solomon,prechargep1: Length of deselect period (phase 1) in
> > clock cycles.
> > +  - solomon,prechargep2: Length of precharge period (phase 2) in
> > clock cycles.
> > +                         This needs to be the higher, the higher
> > the capacitance
> > +                         of the OLED's pixels is
> >  
> >  [0]: Documentation/devicetree/bindings/pwm/pwm.txt
> >  
> > @@ -26,3 +36,14 @@ ssd1307: oled@3c {
> >          reset-gpios = <&gpio2 7>;
> >          reset-active-low;
> >  };
> > +
> > +ssd1306: oled@3c {
> > +        compatible = "solomon,ssd1306fb-i2c";
> > +        reg = <0x3c>;
> > +        pwms = <&pwm 4 3000>;
> > +        reset-gpios = <&gpio2 7>;
> > +        reset-active-low;
> > +        solomon,com-lrremap;
> > +        solomon,com-invdir;
> > +        solomon,com-offset = <32>;
> > +};
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 8d34c56..d16aad4 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -40,20 +40,34 @@
> >  
> >  struct ssd1307fb_par;
> >  
> > -struct ssd1307fb_ops {
> > -	int (*init)(struct ssd1307fb_par *);
> > -	int (*remove)(struct ssd1307fb_par *);
> > +struct ssd1307fb_deviceinfo {
> > +	u32 default_vcomh;
> > +	u32 default_dclk_div;
> > +	u32 default_dclk_frq;
> > +	int need_pwm;
> > +	int need_chargepump;
> >  };
> >  
> >  struct ssd1307fb_par {
> > +	u32 com_invdir;
> > +	u32 com_lrremap;
> > +	u32 com_offset;
> > +	u32 com_seq;
> > +	u32 contrast;
> > +	u32 dclk_div;
> > +	u32 dclk_frq;
> > +	struct ssd1307fb_deviceinfo *device_info;
> >  	struct i2c_client *client;
> >  	u32 height;
> >  	struct fb_info *info;
> > -	struct ssd1307fb_ops *ops;
> >  	u32 page_offset;
> > +	u32 prechargep1;
> > +	u32 prechargep2;
> >  	struct pwm_device *pwm;
> >  	u32 pwm_period;
> >  	int reset;
> > +	u32 seg_remap;
> > +	u32 vcomh;
> >  	u32 width;
> >  };
> >  
> > @@ -254,69 +268,46 @@ static struct fb_deferred_io ssd1307fb_defio
> > = { .deferred_io	= ssd1307fb_deferred_io,
> >  };
> >  
> > -static int ssd1307fb_ssd1307_init(struct ssd1307fb_par *par)
> > +static int ssd1307fb_init(struct ssd1307fb_par *par)
> >  {
> >  	int ret;
> > +	u32 precharge, dclk, com_invdir, compins;
> >  
> > -	par->pwm = pwm_get(&par->client->dev, NULL);
> > -	if (IS_ERR(par->pwm)) {
> > -		dev_err(&par->client->dev, "Could not get PWM from
> > device tree!\n");
> > -		return PTR_ERR(par->pwm);
> > -	}
> > -
> > -	par->pwm_period = pwm_get_period(par->pwm);
> > -	/* Enable the PWM */
> > -	pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
> > -	pwm_enable(par->pwm);
> > -
> > -	dev_dbg(&par->client->dev, "Using PWM%d with a %dns
> > period.\n",
> > -		par->pwm->pwm, par->pwm_period);
> > -
> > -	/* Map column 127 of the OLED to segment 0 */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SEG_REMAP_ON);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	/* Turn on the display */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_DISPLAY_ON);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	return 0;
> > -}
> > -
> > -static int ssd1307fb_ssd1307_remove(struct ssd1307fb_par *par)
> > -{
> > -	pwm_disable(par->pwm);
> > -	pwm_put(par->pwm);
> > -	return 0;
> > -}
> > +	if (par->device_info->need_pwm) {
> > +		par->pwm = pwm_get(&par->client->dev, NULL);
> > +		if (IS_ERR(par->pwm)) {
> > +			dev_err(&par->client->dev, "Could not get
> > PWM from device tree!\n");
> > +			return PTR_ERR(par->pwm);
> > +		}
> >  
> > -static struct ssd1307fb_ops ssd1307fb_ssd1307_ops = {
> > -	.init	= ssd1307fb_ssd1307_init,
> > -	.remove	= ssd1307fb_ssd1307_remove,
> > -};
> > +		par->pwm_period = pwm_get_period(par->pwm);
> > +		/* Enable the PWM */
> > +		pwm_config(par->pwm, par->pwm_period / 2,
> > par->pwm_period);
> > +		pwm_enable(par->pwm);
> >  
> > -static int ssd1307fb_ssd1306_init(struct ssd1307fb_par *par)
> > -{
> > -	int ret;
> > +		dev_dbg(&par->client->dev, "Using PWM%d with a
> > %dns period.\n",
> > +			par->pwm->pwm, par->pwm_period);
> > +	};
> >  
> >  	/* Set initial contrast */
> >  	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_CONTRAST);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x7f);
> > -	if (ret < 0)
> > -		return ret;
> > -
> > -	/* Set COM direction */
> > -	ret = ssd1307fb_write_cmd(par->client, 0xc8);
> > +	ret = ssd1307fb_write_cmd(par->client, par->contrast);
> >  	if (ret < 0)
> >  		return ret;
> >  
> >  	/* Set segment re-map */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SEG_REMAP_ON);
> > +	if (par->seg_remap) {
> > +		ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SEG_REMAP_ON);
> > +		if (ret < 0)
> > +			return ret;
> > +	};
> > +
> > +	/* Set COM direction */
> > +	com_invdir = 0xc0 | (par->com_invdir & 0xf) << 3;
> > +	ret = ssd1307fb_write_cmd(par->client,  com_invdir);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -334,7 +325,7 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x20);
> > +	ret = ssd1307fb_write_cmd(par->client, par->com_offset);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -343,7 +334,8 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0xf0);
> > +	dclk = (par->dclk_div & 0xf) | (par->dclk_frq & 0xf) << 4;
> > +	ret = ssd1307fb_write_cmd(par->client, dclk);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -352,7 +344,8 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x22);
> > +	precharge = (par->prechargep1 & 0xf) | (par->prechargep2 &
> > 0xf) << 4;
> > +	ret = ssd1307fb_write_cmd(par->client, precharge);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -361,7 +354,9 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x22);
> > +	compins = 0x02 | (!par->com_seq & 0x1) << 4
> > +				   | (par->com_lrremap & 0x1) << 5;
> > +	ret = ssd1307fb_write_cmd(par->client, compins);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -370,18 +365,20 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x49);
> > +	ret = ssd1307fb_write_cmd(par->client, par->vcomh);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	/* Turn on the DC-DC Charge Pump */
> > -	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_CHARGE_PUMP);
> > -	if (ret < 0)
> > -		return ret;
> > +	if (par->device_info->need_chargepump) {
> > +		/* Turn on the DC-DC Charge Pump */
> > +		ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_CHARGE_PUMP);
> > +		if (ret < 0)
> > +			return ret;
> >  
> > -	ret = ssd1307fb_write_cmd(par->client, 0x14);
> > -	if (ret < 0)
> > -		return ret;
> > +		ret = ssd1307fb_write_cmd(par->client, 0x14);
> > +		if (ret < 0)
> > +			return ret;
> > +	};
> >  
> >  	/* Switch to horizontal addressing mode */
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_ADDRESS_MODE); @@ -393,6 +390,7 @@ static int
> > ssd1307fb_ssd1306_init(struct ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > +	/* Set column range */
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_COL_RANGE); if (ret < 0)
> >  		return ret;
> > @@ -405,6 +403,7 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) if (ret < 0)
> >  		return ret;
> >  
> > +	/* Set page range */
> >  	ret = ssd1307fb_write_cmd(par->client,
> > SSD1307FB_SET_PAGE_RANGE); if (ret < 0)
> >  		return ret;
> > @@ -426,18 +425,30 @@ static int ssd1307fb_ssd1306_init(struct
> > ssd1307fb_par *par) return 0;
> >  }
> >  
> > -static struct ssd1307fb_ops ssd1307fb_ssd1306_ops = {
> > -	.init	= ssd1307fb_ssd1306_init,
> > +static struct ssd1307fb_deviceinfo ssd1307fb_ssd1306_deviceinfo = {
> > +	.default_vcomh = 0x20,
> > +	.default_dclk_div = 0,
> 
> A division by 0 ?

default_dclk_div stores the bit value for the divider register. The
bit encodes the divider as follows: bit = (divider - 1). But I guess
your are right that it makes more sense to store the actual divider
here and just calculate the bit value when writing to the device.

> 
> > +	.default_dclk_frq = 8,
> > +	.need_pwm = 0,
> 
> You don't need to declare this field, its value will already be
> initialized to 0.
I wasn't sure whether this is necessarily the case. But your hint and
a bit more reading on initialization of static structs convinced me. I
will remove the unnecessary fields.

Thomas

^ permalink raw reply

* Re: [PATCH v1 02/47] x86: mtrr: generalize run time disabling of MTRR
From: Konrad Rzeszutek Wilk @ 2015-03-25 19:59 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: luto, mingo, tglx, hpa, jgross, JBeulich, bp, suresh.b.siddha,
	venkatesh.pallipadi, airlied, linux-kernel, linux-fbdev, x86,
	xen-devel, Luis R. Rodriguez, Ingo Molnar, Daniel Vetter,
	Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Dave Hansen, Stefan Bader, ville.syrjala, david.vrabel,
	toshi.kani, bhelgaas, Roger Pau Monné, xen-devel
In-Reply-To: <1426893517-2511-3-git-send-email-mcgrof@do-not-panic.com>

On Fri, Mar 20, 2015 at 04:17:52PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> It is possible to enable CONFIG_MTRR and up with it
> disabled at run time and yet CONFIG_X86_PAT continues
> to kick through fully functionally. This can happen

s/fully/full/ ?


> for instance on Xen where MTRR is not supported but
> PAT is, this can happen now on Linux as of commit
> 47591df50 by Juergen introduced as of v3.19.

s/3.19/4.0/
> 
> Technically we should assume the proper CPU
> bits would be set to disable MTRR but we can't
> always rely on this. At least on the Xen Hypervisor
> for instance only X86_FEATURE_MTRR was disabled
> as of Xen 4.4 through Xen commit 586ab6a [0],
> but not X86_FEATURE_K6_MTRR, X86_FEATURE_CENTAUR_MCR,
> or X86_FEATURE_CYRIX_ARR for instance.

Oh, could you send an patch for that to Xen please?
> 
> x86 mtrr code relies on quite a bit of checks for
> mtrr_if being set to check to see if MTRR did get
> set up, instead of using that lets provide a generic
> setter which when set we know MTRR is enabled. This

s/we know MTRR is enabled/will let us know that MTRR is enabled/

> also adds a few checks where they were not before
> which could potentially safeguard ourselves against
> incorrect usage of MTRR where this was not desirable.
> 
> Where possible match error codes as if MTRR was
> disabled on arch/x86/include/asm/mtrr.h.
> 
> Lastly, since disabling MTRR can happen at run time
> and we could end up with PAT enabled best record now
> on our logs when MTRR is disabled.
> 
> [0] ~/devel/xen (git::stable-4.5)$ git describe --contains 586ab6a
> 4.4.0-rc1~18
> 
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: venkatesh.pallipadi@intel.com
> Cc: Stefan Bader <stefan.bader@canonical.com>
> Cc: konrad.wilk@oracle.com
> Cc: ville.syrjala@linux.intel.com
> Cc: david.vrabel@citrix.com
> Cc: jbeulich@suse.com
> Cc: toshi.kani@hp.com
> Cc: bhelgaas@google.com
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: xen-devel@lists.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  arch/x86/include/asm/mtrr.h        |  2 ++
>  arch/x86/kernel/cpu/mtrr/cleanup.c |  2 +-
>  arch/x86/kernel/cpu/mtrr/generic.c |  5 +++--
>  arch/x86/kernel/cpu/mtrr/if.c      |  3 +++
>  arch/x86/kernel/cpu/mtrr/main.c    | 31 ++++++++++++++++++++++---------
>  5 files changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/mtrr.h b/arch/x86/include/asm/mtrr.h
> index f768f62..cade917 100644
> --- a/arch/x86/include/asm/mtrr.h
> +++ b/arch/x86/include/asm/mtrr.h
> @@ -31,6 +31,7 @@
>   * arch_phys_wc_add and arch_phys_wc_del.
>   */
>  # ifdef CONFIG_MTRR
> +extern int mtrr_enabled;
>  extern u8 mtrr_type_lookup(u64 addr, u64 end);
>  extern void mtrr_save_fixed_ranges(void *);
>  extern void mtrr_save_state(void);
> @@ -50,6 +51,7 @@ extern int mtrr_trim_uncached_memory(unsigned long end_pfn);
>  extern int amd_special_default_mtrr(void);
>  extern int phys_wc_to_mtrr_index(int handle);
>  #  else
> +static const int mtrr_enabled;
>  static inline u8 mtrr_type_lookup(u64 addr, u64 end)
>  {
>  	/*
> diff --git a/arch/x86/kernel/cpu/mtrr/cleanup.c b/arch/x86/kernel/cpu/mtrr/cleanup.c
> index 5f90b85..784dc55 100644
> --- a/arch/x86/kernel/cpu/mtrr/cleanup.c
> +++ b/arch/x86/kernel/cpu/mtrr/cleanup.c
> @@ -880,7 +880,7 @@ int __init mtrr_trim_uncached_memory(unsigned long end_pfn)
>  	 * Make sure we only trim uncachable memory on machines that
>  	 * support the Intel MTRR architecture:
>  	 */
> -	if (!is_cpu(INTEL) || disable_mtrr_trim)
> +	if (!is_cpu(INTEL) || disable_mtrr_trim || !mtrr_enabled)
>  		return 0;
>  
>  	rdmsr(MSR_MTRRdefType, def, dummy);
> diff --git a/arch/x86/kernel/cpu/mtrr/generic.c b/arch/x86/kernel/cpu/mtrr/generic.c
> index 09c82de..df321b2 100644
> --- a/arch/x86/kernel/cpu/mtrr/generic.c
> +++ b/arch/x86/kernel/cpu/mtrr/generic.c
> @@ -116,7 +116,8 @@ static u8 __mtrr_type_lookup(u64 start, u64 end, u64 *partial_end, int *repeat)
>  	u8 prev_match, curr_match;
>  
>  	*repeat = 0;
> -	if (!mtrr_state_set)
> +	/* generic_mtrr_ops is only set for generic_mtrr_ops */
> +	if (!mtrr_state_set || !mtrr_enabled)
>  		return 0xFF;
>  
>  	if (!mtrr_state.enabled)
> @@ -290,7 +291,7 @@ static void get_fixed_ranges(mtrr_type *frs)
>  
>  void mtrr_save_fixed_ranges(void *info)
>  {
> -	if (cpu_has_mtrr)
> +	if (mtrr_enabled && cpu_has_mtrr)
>  		get_fixed_ranges(mtrr_state.fixed_ranges);
>  }
>  
> diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
> index d76f13d..e9e001a 100644
> --- a/arch/x86/kernel/cpu/mtrr/if.c
> +++ b/arch/x86/kernel/cpu/mtrr/if.c
> @@ -436,6 +436,9 @@ static int __init mtrr_if_init(void)
>  {
>  	struct cpuinfo_x86 *c = &boot_cpu_data;
>  
> +	if (!mtrr_enabled)
> +		return 0;
> +
>  	if ((!cpu_has(c, X86_FEATURE_MTRR)) &&
>  	    (!cpu_has(c, X86_FEATURE_K6_MTRR)) &&
>  	    (!cpu_has(c, X86_FEATURE_CYRIX_ARR)) &&
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index ea5f363..7db9c47 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -59,6 +59,7 @@
>  #define MTRR_TO_PHYS_WC_OFFSET 1000
>  
>  u32 num_var_ranges;
> +int mtrr_enabled;
>  
>  unsigned int mtrr_usage_table[MTRR_MAX_VAR_RANGES];
>  static DEFINE_MUTEX(mtrr_mutex);
> @@ -84,6 +85,9 @@ static int have_wrcomb(void)
>  {
>  	struct pci_dev *dev;
>  
> +	if (!mtrr_enabled)
> +		return 0;
> +
>  	dev = pci_get_class(PCI_CLASS_BRIDGE_HOST << 8, NULL);
>  	if (dev != NULL) {
>  		/*
> @@ -286,7 +290,7 @@ int mtrr_add_page(unsigned long base, unsigned long size,
>  	int i, replace, error;
>  	mtrr_type ltype;
>  
> -	if (!mtrr_if)
> +	if (!mtrr_enabled)
>  		return -ENXIO;
>  
>  	error = mtrr_if->validate_add_page(base, size, type);
> @@ -388,6 +392,8 @@ int mtrr_add_page(unsigned long base, unsigned long size,
>  
>  static int mtrr_check(unsigned long base, unsigned long size)
>  {
> +	if (!mtrr_enabled)
> +		return -ENODEV;
>  	if ((base & (PAGE_SIZE - 1)) || (size & (PAGE_SIZE - 1))) {
>  		pr_warning("mtrr: size and base must be multiples of 4 kiB\n");
>  		pr_debug("mtrr: size: 0x%lx  base: 0x%lx\n", size, base);
> @@ -463,8 +469,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
>  	unsigned long lbase, lsize;
>  	int error = -EINVAL;
>  
> -	if (!mtrr_if)
> -		return -ENXIO;
> +	if (!mtrr_enabled)
> +		return -ENODEV;
>  
>  	max = num_var_ranges;
>  	/* No CPU hotplug when we change MTRR entries */
> @@ -523,6 +529,8 @@ int mtrr_del_page(int reg, unsigned long base, unsigned long size)
>   */
>  int mtrr_del(int reg, unsigned long base, unsigned long size)
>  {
> +	if (!mtrr_enabled)
> +		return -ENODEV;
>  	if (mtrr_check(base, size))
>  		return -EINVAL;
>  	return mtrr_del_page(reg, base >> PAGE_SHIFT, size >> PAGE_SHIFT);
> @@ -545,7 +553,7 @@ int arch_phys_wc_add(unsigned long base, unsigned long size)
>  {
>  	int ret;
>  
> -	if (pat_enabled)
> +	if (pat_enabled || !mtrr_enabled)
>  		return 0;  /* Success!  (We don't need to do anything.) */
>  
>  	ret = mtrr_add(base, size, MTRR_TYPE_WRCOMB, true);
> @@ -734,6 +742,7 @@ void __init mtrr_bp_init(void)
>  	}
>  
>  	if (mtrr_if) {
> +		mtrr_enabled = true;
>  		set_num_var_ranges();
>  		init_table();
>  		if (use_intel()) {
> @@ -744,12 +753,13 @@ void __init mtrr_bp_init(void)
>  				mtrr_if->set_all();
>  			}
>  		}
> -	}
> +	} else
> +		pr_info("mtrr: system does not support MTRR\n");

 'pr_warn' ? 
>  }
>  
>  void mtrr_ap_init(void)
>  {
> -	if (!use_intel() || mtrr_aps_delayed_init)
> +	if (!use_intel() || mtrr_aps_delayed_init || !mtrr_enabled)
>  		return;
>  	/*
>  	 * Ideally we should hold mtrr_mutex here to avoid mtrr entries
> @@ -774,6 +784,9 @@ void mtrr_save_state(void)
>  {
>  	int first_cpu;
>  
> +	if (!mtrr_enabled)
> +		return;
> +
>  	get_online_cpus();
>  	first_cpu = cpumask_first(cpu_online_mask);
>  	smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
> @@ -782,7 +795,7 @@ void mtrr_save_state(void)
>  
>  void set_mtrr_aps_delayed_init(void)
>  {
> -	if (!use_intel())
> +	if (!use_intel() || !mtrr_enabled)
>  		return;
>  
>  	mtrr_aps_delayed_init = true;
> @@ -810,7 +823,7 @@ void mtrr_aps_init(void)
>  
>  void mtrr_bp_restore(void)
>  {
> -	if (!use_intel())
> +	if (!use_intel() || !mtrr_enabled)
>  		return;
>  
>  	mtrr_if->set_all();
> @@ -818,7 +831,7 @@ void mtrr_bp_restore(void)
>  
>  static int __init mtrr_init_finialize(void)
>  {
> -	if (!mtrr_if)
> +	if (!mtrr_enabled)
>  		return 0;
>  
>  	if (use_intel()) {
> -- 
> 2.3.2.209.gd67f9d5.dirty
> 

^ permalink raw reply

* Re: [PATCH v1 03/47] devres: add devm_ioremap_wc()
From: Luis R. Rodriguez @ 2015-03-25 19:50 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Luis R. Rodriguez, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Juergen Gross, Jan Beulich, Borislav Petkov, Suresh Siddha,
	venkatesh.pallipadi, Dave Airlie, linux-kernel@vger.kernel.org,
	Linux Fbdev development list, X86 ML,
	xen-devel@lists.xenproject.org, Ingo Molnar, Daniel Vetter,
	Antonino Daplas, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <CALCETrX5K4pZE0DqcEwJiKYZofCr6yjJGK8MhjhPXMHNLJ4TTQ@mail.gmail.com>

On Fri, Mar 20, 2015 at 04:49:51PM -0700, Andy Lutomirski wrote:
> On Fri, Mar 20, 2015 at 4:17 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > We have devm_ioremap_nocache() but no devm_ioremap_wc()
> > so add that. This will be used later.
> >
> > Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> 
> Looks good to me.

Thanks, I'll peg a Reviewed-by.

 Luis

^ permalink raw reply

* Re: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Maxime Ripard @ 2015-03-25 16:02 UTC (permalink / raw)
  To: Olliver Schinagl
  Cc: Tomi Valkeinen, Thomas Niederprüm, linux-fbdev, plagnioj,
	linux-kernel
In-Reply-To: <55129466.4060000@schinagl.nl>

[-- Attachment #1: Type: text/plain, Size: 3885 bytes --]

On Wed, Mar 25, 2015 at 11:56:38AM +0100, Olliver Schinagl wrote:
> Hey all,
> 
> On 10-03-15 11:45, Tomi Valkeinen wrote:
> >On 14/02/15 17:54, Maxime Ripard wrote:
> >>On Sat, Feb 07, 2015 at 05:05:03PM +0100, Thomas Niederprüm wrote:
> >>>Am Sat, 7 Feb 2015 12:20:43 +0100
> >>>schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> >>>
> >>>>On Fri, Feb 06, 2015 at 11:28:11PM +0100, niederp@physik.uni-kl.de
> >>>>wrote:
> >>>>>From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> >>>>>
> >>>>>This patch adds a module parameter 'bitsperpixel' to adjust the
> >>>>>colordepth of the framebuffer. All values >1 will result in memory
> >>>>>map of the requested color depth. However only the MSB of each
> >>>>>pixel will be sent to the device. The framebuffer identifies itself
> >>>>>as a grayscale display with the specified depth.
> >>>>
> >>>>I'm not sure this is the right thing to do.
> >>>>
> >>>>The bits per pixel for this display is rightfully defined, used and
> >>>>reported to the userspace, why would you want to change that?
> >>>
> >>>You are right of course. The display is 1bpp and it reports to be 1
> >>>bpp. The problem is that there is almost no userspace library that can
> >>>handle 1 bit framebuffers correctly. So it is nice if the framebuffer
> >>>(optionally) can expose itself as 8 bits per pixel grayscale to the
> >>>userspace program. As an example this allows to run DirectFB on the
> >>>framebuffer, which is not possible out of the box for 1bpp.
> >>>
> >>>Also note that if do not set the module parameter at load time
> >>>the framebuffer will be 1bpp. So you have to actively set that module
> >>>parameter to make the framebuffer pretend to be more than 1bpp.
> >>>
> >>>In any case I don't cling to that patch, I just thought it was a nice
> >>>feature.
> >>
> >>I'd say that the right fix would be to patch DirectFB, instead of
> >>faking that in the kernel.
> >>
> >>But again, that's probably Tomi's call, not mine.
> >
> >Right, I'm not thrilled =). I don't think it's a good idea to lie to the
> >userspace (except when fixing regressions).
> 
> I've done the same thing actually in a local patchset and while you are
> right, we shouldn't lie to userspace, my choice was a performance one.

Userspace drivers can be more performant than the kernel ones. That
doesn't mean that it's a good choice, let alone the right choice.

> Right now, in the driver we already have to convert from a regular packed
> pixel framebuffer, to the format that supports the page layout of the actual
> chip. Especially on slow hardware, doing the math within this conversion
> just adds a few multiplications.

And how does implementing monochrome operations in directfb would make
things worse from a performance point of view?

Are you really using performance as an argument for a device that is
driven through i2c?

> Also, there is indeed a lot of userspace out there which doesn't expect
> single bit displays.

And so just because *they* don't expect it and are not supporting
every case they should, *we* should do something about it?

A lot of userspace applications don't check syscall errors. Does that
mean that we should never return an error?

> Having said that, what about actually faking grayscale? If we toggle a pixel
> fast enough (we can achieve 40ish fps right now with a 400 kHz I2C bus) it
> appears to a user as gray. This can't easily be done in user space, would
> that be acceptable?

You can't make the assumption that the bus is fast enough. It's your
case, it might not be the case for everyone.

Seriously, the device is monochrome, deal with it. If you wanted a
grayscale screen, you should have chosen better.

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox