Fw: Rivafb I2C problem, new patch

Fw: Rivafb I2C problem, new patch

[Andrew Morton]

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


Maybe Greg, or the linux-fbdev-devel denizens can comment upon
this?


Begin forwarded message:

Date: Tue, 13 Jul 2004 01:24:02 +0200
From: Nicolas Boichat <nicolas@boichat.ch>
To: akpm@digeo.com
Subject: Rivafb I2C problem, new patch


Hello,

I'm currently writing a DDC/CI tool for Linux (DDC/CI is the protocol which permits to modify monitor parameters without using the OSD), see http://www.boichat.ch/nicolas/ddcci .

I use the new rivafb I2C support, but a problem appeared: the I2C bus is immediately deleted after the DDC2 EDID is read, but I need to access it from my application!

I attached a patch which fixes this problem: the bus is deleted only when the driver is unloaded (please note that the atyfb behave like my patched version). Could you add it to the next -mm kernel release ?

I also contacted the driver maintainer but he seems not to answer (I sent an e-mail to him about a week ago).

Best regards,

Nicolas Boichat


[-- Attachment #2: riva-2.6.7-mm6.patch --]
[-- Type: text/x-patch, Size: 1233 bytes --]

--- linux-2.6.7/drivers/video/riva.old/fbdev.c	2004-07-07 11:26:23.000000000 +0200
+++ linux-2.6.7/drivers/video/riva/fbdev.c	2004-07-07 11:36:47.008371568 +0200
@@ -1773,7 +1773,7 @@
 	struct riva_par *par = (struct riva_par *) info->par;
 	int i;
 
-	riva_create_i2c_busses(par);
+	//riva_create_i2c_busses(par);
 	for (i = par->bus; i >= 1; i--) {
 		riva_probe_i2c_connector(par, i, &par->EDID);
 		if (par->EDID) {
@@ -1781,7 +1781,7 @@
 			break;
 		}
 	}
-	riva_delete_i2c_busses(par);
+	//riva_delete_i2c_busses(par);
 #endif
 #endif
 }
@@ -1933,6 +1933,8 @@
 	}
 #endif /* CONFIG_MTRR */
 
+	riva_create_i2c_busses((struct riva_par *) info->par);
+
 	info->fbops = &riva_fb_ops;
 	info->fix = rivafb_fix;
 	riva_get_EDID(info, pd);
@@ -1961,6 +1963,7 @@
 	return 0;
 
 err_out_iounmap_fb:
+	riva_delete_i2c_busses((struct riva_par *) info->par);
 	iounmap(info->screen_base);
 err_out_free_base1:
 	if (default_par->riva.Architecture == NV_ARCH_03) 
@@ -1985,10 +1988,12 @@
 {
 	struct fb_info *info = pci_get_drvdata(pd);
 	struct riva_par *par = (struct riva_par *) info->par;
-	
+
 	if (!info)
 		return;
 
+	riva_delete_i2c_busses(par);
+
 	unregister_framebuffer(info);
 #ifdef CONFIG_MTRR
 	if (par->mtrr.vram_valid)

Re: Fw: Rivafb I2C problem, new patch

[Greg KH]

On Mon, Jul 12, 2004 at 04:39:42PM -0700, Andrew Morton wrote:
> 
> Maybe Greg, or the linux-fbdev-devel denizens can comment upon
> this?
> 
> 
> Begin forwarded message:
> 
> Date: Tue, 13 Jul 2004 01:24:02 +0200
> From: Nicolas Boichat <nicolas@boichat.ch>
> To: akpm@digeo.com
> Subject: Rivafb I2C problem, new patch
> 
> 
> Hello,
> 
> I'm currently writing a DDC/CI tool for Linux (DDC/CI is the protocol
> which permits to modify monitor parameters without using the OSD), see
> http://www.boichat.ch/nicolas/ddcci .
> 
> I use the new rivafb I2C support, but a problem appeared: the I2C bus
> is immediately deleted after the DDC2 EDID is read, but I need to
> access it from my application!
> 
> I attached a patch which fixes this problem: the bus is deleted only
> when the driver is unloaded (please note that the atyfb behave like my
> patched version). Could you add it to the next -mm kernel release ?

Well, you shouldn't comment out those calls, just delete them.

Other than that, from an i2c standpoint, the patch looks sane, but I
would check with the maintainer, as there might be some reason they
delete the bus right after using it.

thanks,

greg k-h


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com

Re: Fw: Rivafb I2C problem, new patch

[Antonino A. Daplas]

On Tuesday 13 July 2004 07:39, Andrew Morton wrote:
> Maybe Greg, or the linux-fbdev-devel denizens can comment upon
> this?
>
>
> Begin forwarded message:
>
> Date: Tue, 13 Jul 2004 01:24:02 +0200
> From: Nicolas Boichat <nicolas@boichat.ch>
> To: akpm@digeo.com
> Subject: Rivafb I2C problem, new patch
>
>
> Hello,
>
> I'm currently writing a DDC/CI tool for Linux (DDC/CI is the protocol which
> permits to modify monitor parameters without using the OSD), see
> http://www.boichat.ch/nicolas/ddcci .
>
> I use the new rivafb I2C support, but a problem appeared: the I2C bus is
> immediately deleted after the DDC2 EDID is read, but I need to access it
> from my application!
>
> I attached a patch which fixes this problem: the bus is deleted only when
> the driver is unloaded (please note that the atyfb behave like my patched
> version). Could you add it to the next -mm kernel release ?
>
> I also contacted the driver maintainer but he seems not to answer (I sent
> an e-mail to him about a week ago).

I'm not the maintainer of rivafb but I added the DDC2 support for it. The bus
was destroyed because there was no use for it after reading the EDID block,
and fbdev has no interface (yet) to re-read it again.

It doesn't matter to me whether the bus is immediately deleted or not, so the
patch is fine by me.

Tony 




-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com

Re: Fw: Rivafb I2C problem, new patch

[Nicolas Boichat]

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

Hello,

Thanks for your answers.

I attached a new patch: I deleted the lines that were commented out.

Best regards,

Nicolas

On Tue, 2004-07-13 at 03:58, Antonino A. Daplas wrote:
> I'm not the maintainer of rivafb but I added the DDC2 support for it. The bus
> was destroyed because there was no use for it after reading the EDID block,
> and fbdev has no interface (yet) to re-read it again.
> 
> It doesn't matter to me whether the bus is immediately deleted or not, so the
> patch is fine by me.
> 
> Tony 

On Tue, 2004-07-13 at 01:43, Greg KH wrote: 
> Well, you shouldn't comment out those calls, just delete them.
> 
> Other than that, from an i2c standpoint, the patch looks sane, but I
> would check with the maintainer, as there might be some reason they
> delete the bus right after using it.
> 
> thanks,
> 
> greg k-h

[-- Attachment #2: Type: text/x-patch, Size: 1042 bytes --]

--- linux/drivers/video/riva/fbdev.c.old	2004-07-13 02:05:47.029754640 +0200
+++ linux/drivers/video/riva/fbdev.c	2004-07-13 02:08:25.945966745 +0200
@@ -1773,7 +1773,6 @@
 	struct riva_par *par = (struct riva_par *) info->par;
 	int i;
 
-	riva_create_i2c_busses(par);
 	for (i = par->bus; i >= 1; i--) {
 		riva_probe_i2c_connector(par, i, &par->EDID);
 		if (par->EDID) {
@@ -1781,7 +1780,6 @@
 			break;
 		}
 	}
-	riva_delete_i2c_busses(par);
 #endif
 #endif
 }
@@ -1933,6 +1931,8 @@
 	}
 #endif /* CONFIG_MTRR */
 
+	riva_create_i2c_busses((struct riva_par *) info->par);
+
 	info->fbops = &riva_fb_ops;
 	info->fix = rivafb_fix;
 	riva_get_EDID(info, pd);
@@ -1961,6 +1961,7 @@
 	return 0;
 
 err_out_iounmap_fb:
+	riva_delete_i2c_busses((struct riva_par *) info->par);
 	iounmap(info->screen_base);
 err_out_free_base1:
 	if (default_par->riva.Architecture == NV_ARCH_03) 
@@ -1989,6 +1990,8 @@
 	if (!info)
 		return;
 
+	riva_delete_i2c_busses(par);
+
 	unregister_framebuffer(info);
 #ifdef CONFIG_MTRR
 	if (par->mtrr.vram_valid)

Re: Fw: Rivafb I2C problem, new patch

[Antonino A. Daplas]

On Tuesday 13 July 2004 17:53, Nicolas Boichat wrote:
> Hello,
>
> Thanks for your answers.
>
> I attached a new patch: I deleted the lines that were commented out.
>

One thing I forgot, make sure it compiles when CONFIG_FB_RIVA_I2C is
set or not.

Tony




-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com

Re: Fw: Rivafb I2C problem, new patch

[Nicolas Boichat]

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

Hello,

Good idea... it didn't compile. Fixed patch attached.

Best regards,

Nicolas

On Tue, 2004-07-13 at 22:52, Antonino A. Daplas wrote:
> On Tuesday 13 July 2004 17:53, Nicolas Boichat wrote:
> > Hello,
> >
> > Thanks for your answers.
> >
> > I attached a new patch: I deleted the lines that were commented out.
> >
> 
> One thing I forgot, make sure it compiles when CONFIG_FB_RIVA_I2C is
> set or not.
> 
> Tony
> 
> 
> 

[-- Attachment #2: Type: text/x-patch, Size: 1099 bytes --]

--- fbdev.c.old	2004-07-13 02:05:47.000000000 +0200
+++ fbdev.c	2004-07-13 23:34:27.023199272 +0200
@@ -1773,7 +1773,6 @@
 	struct riva_par *par = (struct riva_par *) info->par;
 	int i;
 
-	riva_create_i2c_busses(par);
 	for (i = par->bus; i >= 1; i--) {
 		riva_probe_i2c_connector(par, i, &par->EDID);
 		if (par->EDID) {
@@ -1781,7 +1780,6 @@
 			break;
 		}
 	}
-	riva_delete_i2c_busses(par);
 #endif
 #endif
 }
@@ -1933,6 +1931,10 @@
 	}
 #endif /* CONFIG_MTRR */
 
+#ifdef CONFIG_FB_RIVA_I2C
+	riva_create_i2c_busses((struct riva_par *) info->par);
+#endif
+
 	info->fbops = &riva_fb_ops;
 	info->fix = rivafb_fix;
 	riva_get_EDID(info, pd);
@@ -1961,6 +1963,9 @@
 	return 0;
 
 err_out_iounmap_fb:
+#ifdef CONFIG_FB_RIVA_I2C
+	riva_delete_i2c_busses((struct riva_par *) info->par);
+#endif
 	iounmap(info->screen_base);
 err_out_free_base1:
 	if (default_par->riva.Architecture == NV_ARCH_03) 
@@ -1989,6 +1994,10 @@
 	if (!info)
 		return;
 
+#ifdef CONFIG_FB_RIVA_I2C
+	riva_delete_i2c_busses(par);
+#endif
+
 	unregister_framebuffer(info);
 #ifdef CONFIG_MTRR
 	if (par->mtrr.vram_valid)

Re: Fw: Rivafb I2C problem, new patch

[Andrew Morton]

Nicolas Boichat <nicolas@boichat.ch> wrote:
>
> Good idea... it didn't compile. Fixed patch attached.

Please send a complete new patch, with a changelog.

Thanks.


-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - 
digital self defense, top technical experts, no vendor pitches, 
unmatched networking opportunities. Visit www.blackhat.com

Re: Fw: Rivafb I2C problem, new patch

[Nicolas Boichat]

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

Hello,

This should be the last patch, the changelog is at the beginning of the
file.

Best regards,

Nicolas

On Wed, 2004-07-14 at 00:02, Andrew Morton wrote:
> Nicolas Boichat <nicolas@boichat.ch> wrote:
> >
> > Good idea... it didn't compile. Fixed patch attached.
> 
> Please send a complete new patch, with a changelog.
> 
> Thanks.
> 

[-- Attachment #2: rivafb-i2c-bus-fix.patch --]
[-- Type: text/x-patch, Size: 1795 bytes --]

From: Nicolas Boichat <nicolas@boichat.ch>

The I2C busses opened by rivafb were deleted immediately after
reading the EDID, but they should be kept open, so user-space
applications can use them.

They are now deleted when the driver is unloaded.

---

 linux/drivers/video/riva/fbdev.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff -puN linux/drivers/video/riva/fbdev.c.old  linux/drivers/video/riva/fbdev.c
--- linux/drivers/video/riva/fbdev.c.old	2004-07-13 02:05:47.000000000 +0200
+++ linux/drivers/video/riva/fbdev.c	2004-07-13 23:34:27.023199272 +0200
@@ -1773,7 +1773,6 @@ static void riva_get_EDID(struct fb_info
 	struct riva_par *par = (struct riva_par *) info->par;
 	int i;
 
-	riva_create_i2c_busses(par);
 	for (i = par->bus; i >= 1; i--) {
 		riva_probe_i2c_connector(par, i, &par->EDID);
 		if (par->EDID) {
@@ -1781,7 +1780,6 @@ static void riva_get_EDID(struct fb_info
 			break;
 		}
 	}
-	riva_delete_i2c_busses(par);
 #endif
 #endif
 }
@@ -1933,6 +1931,10 @@ static int __devinit rivafb_probe(struct
 	}
 #endif /* CONFIG_MTRR */
 
+#ifdef CONFIG_FB_RIVA_I2C
+	riva_create_i2c_busses((struct riva_par *) info->par);
+#endif
+
 	info->fbops = &riva_fb_ops;
 	info->fix = rivafb_fix;
 	riva_get_EDID(info, pd);
@@ -1961,6 +1963,9 @@ static int __devinit rivafb_probe(struct
 	return 0;
 
 err_out_iounmap_fb:
+#ifdef CONFIG_FB_RIVA_I2C
+	riva_delete_i2c_busses((struct riva_par *) info->par);
+#endif
 	iounmap(info->screen_base);
 err_out_free_base1:
 	if (default_par->riva.Architecture == NV_ARCH_03) 
@@ -1989,6 +1994,10 @@ static void __exit rivafb_remove(struct 
 	if (!info)
 		return;
 
+#ifdef CONFIG_FB_RIVA_I2C
+	riva_delete_i2c_busses(par);
+#endif
+
 	unregister_framebuffer(info);
 #ifdef CONFIG_MTRR
 	if (par->mtrr.vram_valid)