[patch] rivafb/nvidiafb: race between register_framebuffer and *_bl_init

[patch] rivafb/nvidiafb: race between register_framebuffer and *_bl_init

[Guido Guenther]

Hi,
Since we now use the generic backlight infrastructure, I think we need
to call rivafb_bl_init before calling register_framebuffer since
otherwise rivafb_bl_init might race with the framebuffer layer already
opening the device and setting up the video mode. In this case we might
end up with a not yet fully intialized backlight (info->bl_dev still
NULL) when calling riva_bl_set_power via
rivafb_set_par/rivafb_load_video_mode and the kernel dies without any
further notice during boot.
This fixes booting current git on a PB 6,1, please apply if it looks
correct - in this case radeonfb/atyfb would be affected too - I can fix
that too but don't have any hardware to test this on.
Cheers,
 -- Guido

diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
index 9f2066f..eae291e 100644
--- a/drivers/video/nvidia/nvidia.c
+++ b/drivers/video/nvidia/nvidia.c
@@ -1303,20 +1303,19 @@ #endif				/* CONFIG_MTRR */
 
 	nvidia_save_vga(par, &par->SavedReg);
 
+	pci_set_drvdata(pd, info);
+	nvidia_bl_init(par);
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR PFX "error registering nVidia framebuffer\n");
 		goto err_out_iounmap_fb;
 	}
 
-	pci_set_drvdata(pd, info);
 
 	printk(KERN_INFO PFX
 	       "PCI nVidia %s framebuffer (%dMB @ 0x%lX)\n",
 	       info->fix.id,
 	       par->FbMapSize / (1024 * 1024), info->fix.smem_start);
 
-	nvidia_bl_init(par);
-
 	NVTRACE_LEAVE();
 	return 0;
 
@@ -2132,15 +2132,17 @@ #endif /* CONFIG_MTRR */
 
 	fb_destroy_modedb(info->monspecs.modedb);
 	info->monspecs.modedb = NULL;
+
+	pci_set_drvdata(pd, info);
+	riva_bl_init(info->par);
 	ret = register_framebuffer(info);
+
 	if (ret < 0) {
 		printk(KERN_ERR PFX
 			"error registering riva framebuffer\n");
 		goto err_iounmap_screen_base;
 	}
 
-	pci_set_drvdata(pd, info);
-
 	printk(KERN_INFO PFX
 		"PCI nVidia %s framebuffer ver %s (%dMB @ 0x%lX)\n",
 		info->fix.id,
@@ -2148,8 +2150,6 @@ #endif /* CONFIG_MTRR */
 		info->fix.smem_len / (1024 * 1024),
 		info->fix.smem_start);
 
-	riva_bl_init(info->par);
-
 	NVTRACE_LEAVE();
 	return 0;
 


Signed-Off-By: Guido Guenther <agx@sigxcpu.org>

Re: [patch] rivafb/nvidiafb: race between register_framebuffer and *_bl_init

[Guido Guenther]

Hi,
On Sat, Jul 22, 2006 at 06:28:21PM +0200, Guido Guenther wrote:
> This fixes booting current git on a PB 6,1, please apply if it looks
> correct - in this case radeonfb/atyfb would be affected too - I can fix
> that too but don't have any hardware to test this on.
...no it doesn't because the patch doesn't apply. This one does, sorry
for the noise.
 -- Guido

diff --git a/drivers/video/aty/radeon_base.c b/drivers/video/aty/radeon_base.c
diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
index 9f2066f..eae291e 100644
--- a/drivers/video/nvidia/nvidia.c
+++ b/drivers/video/nvidia/nvidia.c
@@ -1303,20 +1303,19 @@ #endif				/* CONFIG_MTRR */
 
 	nvidia_save_vga(par, &par->SavedReg);
 
+	pci_set_drvdata(pd, info);
+	nvidia_bl_init(par);
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR PFX "error registering nVidia framebuffer\n");
 		goto err_out_iounmap_fb;
 	}
 
-	pci_set_drvdata(pd, info);
 
 	printk(KERN_INFO PFX
 	       "PCI nVidia %s framebuffer (%dMB @ 0x%lX)\n",
 	       info->fix.id,
 	       par->FbMapSize / (1024 * 1024), info->fix.smem_start);
 
-	nvidia_bl_init(par);
-
 	NVTRACE_LEAVE();
 	return 0;
 
diff --git a/drivers/video/riva/fbdev.c b/drivers/video/riva/fbdev.c
index 33dddba..43aa8be 100644
--- a/drivers/video/riva/fbdev.c
+++ b/drivers/video/riva/fbdev.c
@@ -2132,6 +2132,9 @@ #endif /* CONFIG_MTRR */
 
 	fb_destroy_modedb(info->monspecs.modedb);
 	info->monspecs.modedb = NULL;
+
+	pci_set_drvdata(pd, info);
+	riva_bl_init(info->par);
 	ret = register_framebuffer(info);
 	if (ret < 0) {
 		printk(KERN_ERR PFX
@@ -2139,8 +2142,6 @@ #endif /* CONFIG_MTRR */
 		goto err_iounmap_screen_base;
 	}
 
-	pci_set_drvdata(pd, info);
-
 	printk(KERN_INFO PFX
 		"PCI nVidia %s framebuffer ver %s (%dMB @ 0x%lX)\n",
 		info->fix.id,
@@ -2148,8 +2149,6 @@ #endif /* CONFIG_MTRR */
 		info->fix.smem_len / (1024 * 1024),
 		info->fix.smem_start);
 
-	riva_bl_init(info->par);
-
 	NVTRACE_LEAVE();
 	return 0;
 


Signed-Off-By: Guido Guenther <agx@sigxcpu.org>

Re: [patch] rivafb/nvidiafb: race between register_framebuffer and *_bl_init

[Antonino A. Daplas]

Guido Guenther wrote:

Please add your Signed-off-by:

Tony

> Hi,
> On Sat, Jul 22, 2006 at 06:28:21PM +0200, Guido Guenther wrote:
>> This fixes booting current git on a PB 6,1, please apply if it looks
>> correct - in this case radeonfb/atyfb would be affected too - I can fix
>> that too but don't have any hardware to test this on.
> ...no it doesn't because the patch doesn't apply. This one does, sorry
> for the noise.
>  -- Guido
> 
> diff --git a/drivers/video/aty/radeon_base.c b/drivers/video/aty/radeon_base.c
> diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
> index 9f2066f..eae291e 100644
> --- a/drivers/video/nvidia/nvidia.c
> +++ b/drivers/video/nvidia/nvidia.c
> @@ -1303,20 +1303,19 @@ #endif				/* CONFIG_MTRR */
>  
>  	nvidia_save_vga(par, &par->SavedReg);
>  
> +	pci_set_drvdata(pd, info);
> +	nvidia_bl_init(par);
>  	if (register_framebuffer(info) < 0) {
>  		printk(KERN_ERR PFX "error registering nVidia framebuffer\n");
>  		goto err_out_iounmap_fb;
>  	}
>  
> -	pci_set_drvdata(pd, info);
>  
>  	printk(KERN_INFO PFX
>  	       "PCI nVidia %s framebuffer (%dMB @ 0x%lX)\n",
>  	       info->fix.id,
>  	       par->FbMapSize / (1024 * 1024), info->fix.smem_start);
>  
> -	nvidia_bl_init(par);
> -
>  	NVTRACE_LEAVE();
>  	return 0;
>  
> diff --git a/drivers/video/riva/fbdev.c b/drivers/video/riva/fbdev.c
> index 33dddba..43aa8be 100644
> --- a/drivers/video/riva/fbdev.c
> +++ b/drivers/video/riva/fbdev.c
> @@ -2132,6 +2132,9 @@ #endif /* CONFIG_MTRR */
>  
>  	fb_destroy_modedb(info->monspecs.modedb);
>  	info->monspecs.modedb = NULL;
> +
> +	pci_set_drvdata(pd, info);
> +	riva_bl_init(info->par);
>  	ret = register_framebuffer(info);
>  	if (ret < 0) {
>  		printk(KERN_ERR PFX
> @@ -2139,8 +2142,6 @@ #endif /* CONFIG_MTRR */
>  		goto err_iounmap_screen_base;
>  	}
>  
> -	pci_set_drvdata(pd, info);
> -
>  	printk(KERN_INFO PFX
>  		"PCI nVidia %s framebuffer ver %s (%dMB @ 0x%lX)\n",
>  		info->fix.id,
> @@ -2148,8 +2149,6 @@ #endif /* CONFIG_MTRR */
>  		info->fix.smem_len / (1024 * 1024),
>  		info->fix.smem_start);
>  
> -	riva_bl_init(info->par);
> -
>  	NVTRACE_LEAVE();
>  	return 0;
>  
> 
> 
> Signed-Off-By: Guido Guenther <agx@sigxcpu.org>
> 

Re: [patch] rivafb/nvidiafb: race between register_framebuffer and *_bl_init

[Guido Guenther]

On Mon, Jul 24, 2006 at 08:17:38AM +0800, Antonino A. Daplas wrote:
> Guido Guenther wrote:
> 
> Please add your Signed-off-by:
It's at the very bottom of the patch already.
Cheers,
 -- Guido

Re: [patch] rivafb/nvidiafb: race between register_framebuffer and *_bl_init

[Antonino A. Daplas]

Guido Guenther wrote:
> On Mon, Jul 24, 2006 at 08:17:38AM +0800, Antonino A. Daplas wrote:
>> Guido Guenther wrote:
>>
>> Please add your Signed-off-by:
> It's at the very bottom of the patch already.

Okay, my script missed it.  This line should be after the changelog text.

Tony