Patchwork [1/3] UBUNTU: [Upstream] drm/nv04-nv40: Fix up the programmed horizontal sync pulse delay.

login
register
mail settings
Submitter Christopher James Halse Rogers
Date March 25, 2010, 10:25 p.m.
Message ID <1269555956-3532-2-git-send-email-raof@ubuntu.com>
Download mbox | patch
Permalink /patch/48601/
State Accepted
Delegated to: Andy Whitcroft
Headers show

Comments

Christopher James Halse Rogers - March 25, 2010, 10:25 p.m.
From: Christopher James Halse Rogers <raof@ubuntu.com>

The calculated values were a little bit off (~16 clocks), the only
effect it could have had is a slightly offset image with respect to
the blob on analog outputs (bug 26790).

BugLink: http://bugs.launchpad.net/bugs/529130

Signed-off-by: Francisco Jerez <currojerez@riseup.net>
Signed-off-by: Christopher James Halse Rogers <raof@ubuntu.com>
---
 drivers/gpu/drm/nouveau/nv04_crtc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Stefan Bader - March 29, 2010, 12:58 p.m.
Chris got back to me directly and said that this only were two patches to be
sent. He also wanted to pass them on to stable.
@Chris, this is done by just passing on that patch (with the sha1 reference of
the upstream commit to stable@kernel.org. Usually cc'ing maintainers and other
people on the s-o-b list and asking for inclusion.
This one looks ok to me.

Chris Halse Rogers wrote:
> From: Christopher James Halse Rogers <raof@ubuntu.com>
> 
> The calculated values were a little bit off (~16 clocks), the only
> effect it could have had is a slightly offset image with respect to
> the blob on analog outputs (bug 26790).
> 
> BugLink: http://bugs.launchpad.net/bugs/529130
> 
> Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> Signed-off-by: Christopher James Halse Rogers <raof@ubuntu.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  drivers/gpu/drm/nouveau/nv04_crtc.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
> index d2f143e..9986aba 100644
> --- a/drivers/gpu/drm/nouveau/nv04_crtc.c
> +++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
> @@ -230,9 +230,9 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode)
>  	struct drm_framebuffer *fb = crtc->fb;
>  
>  	/* Calculate our timings */
> -	int horizDisplay	= (mode->crtc_hdisplay >> 3) 	- 1;
> -	int horizStart		= (mode->crtc_hsync_start >> 3) 	- 1;
> -	int horizEnd		= (mode->crtc_hsync_end >> 3) 	- 1;
> +	int horizDisplay	= (mode->crtc_hdisplay >> 3)		- 1;
> +	int horizStart		= (mode->crtc_hsync_start >> 3) 	+ 1;
> +	int horizEnd		= (mode->crtc_hsync_end >> 3)		+ 1;
>  	int horizTotal		= (mode->crtc_htotal >> 3)		- 5;
>  	int horizBlankStart	= (mode->crtc_hdisplay >> 3)		- 1;
>  	int horizBlankEnd	= (mode->crtc_htotal >> 3)		- 1;
Andy Whitcroft - March 30, 2010, 1:18 p.m.
On Fri, Mar 26, 2010 at 09:25:55AM +1100, Chris Halse Rogers wrote:
> From: Christopher James Halse Rogers <raof@ubuntu.com>
> 
> The calculated values were a little bit off (~16 clocks), the only
> effect it could have had is a slightly offset image with respect to
> the blob on analog outputs (bug 26790).

I would love to know what the heck a blob is in this context and how I
could tell my image was offset from it :).

> BugLink: http://bugs.launchpad.net/bugs/529130
> 
> Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> Signed-off-by: Christopher James Halse Rogers <raof@ubuntu.com>
> ---
>  drivers/gpu/drm/nouveau/nv04_crtc.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
> index d2f143e..9986aba 100644
> --- a/drivers/gpu/drm/nouveau/nv04_crtc.c
> +++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
> @@ -230,9 +230,9 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode)
>  	struct drm_framebuffer *fb = crtc->fb;
>  
>  	/* Calculate our timings */
> -	int horizDisplay	= (mode->crtc_hdisplay >> 3) 	- 1;
> -	int horizStart		= (mode->crtc_hsync_start >> 3) 	- 1;
> -	int horizEnd		= (mode->crtc_hsync_end >> 3) 	- 1;
> +	int horizDisplay	= (mode->crtc_hdisplay >> 3)		- 1;
> +	int horizStart		= (mode->crtc_hsync_start >> 3) 	+ 1;
> +	int horizEnd		= (mode->crtc_hsync_end >> 3)		+ 1;
>  	int horizTotal		= (mode->crtc_htotal >> 3)		- 5;
>  	int horizBlankStart	= (mode->crtc_hdisplay >> 3)		- 1;
>  	int horizBlankEnd	= (mode->crtc_htotal >> 3)		- 1;

Looks like it might do what it says, and its upstreaming.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Christopher James Halse Rogers - March 30, 2010, 11:26 p.m.
On Tue, 2010-03-30 at 14:18 +0100, Andy Whitcroft wrote:
> On Fri, Mar 26, 2010 at 09:25:55AM +1100, Chris Halse Rogers wrote:
> > From: Christopher James Halse Rogers <raof@ubuntu.com>
> > 
> > The calculated values were a little bit off (~16 clocks), the only
> > effect it could have had is a slightly offset image with respect to
> > the blob on analog outputs (bug 26790).
> 
> I would love to know what the heck a blob is in this context and how I
> could tell my image was offset from it :).
> 
In this context the blob is the nvidia binary drivers.  The difference
in timing meant that for users of VGA outputs the display was offset
with nouveau after fiddling with the monitor to align it correctly when
run with the nvidia drivers.  This would be particularly annoying for
dual-booters, who'd need to re-align their monitor on each OS switch.

I don't miss having to fiddle with image alignment on analogue outputs!

> > BugLink: http://bugs.launchpad.net/bugs/529130
> > 
> > Signed-off-by: Francisco Jerez <currojerez@riseup.net>
> > Signed-off-by: Christopher James Halse Rogers <raof@ubuntu.com>
> > ---
> >  drivers/gpu/drm/nouveau/nv04_crtc.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
> > index d2f143e..9986aba 100644
> > --- a/drivers/gpu/drm/nouveau/nv04_crtc.c
> > +++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
> > @@ -230,9 +230,9 @@ nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode)
> >  	struct drm_framebuffer *fb = crtc->fb;
> >  
> >  	/* Calculate our timings */
> > -	int horizDisplay	= (mode->crtc_hdisplay >> 3) 	- 1;
> > -	int horizStart		= (mode->crtc_hsync_start >> 3) 	- 1;
> > -	int horizEnd		= (mode->crtc_hsync_end >> 3) 	- 1;
> > +	int horizDisplay	= (mode->crtc_hdisplay >> 3)		- 1;
> > +	int horizStart		= (mode->crtc_hsync_start >> 3) 	+ 1;
> > +	int horizEnd		= (mode->crtc_hsync_end >> 3)		+ 1;
> >  	int horizTotal		= (mode->crtc_htotal >> 3)		- 5;
> >  	int horizBlankStart	= (mode->crtc_hdisplay >> 3)		- 1;
> >  	int horizBlankEnd	= (mode->crtc_htotal >> 3)		- 1;
> 
> Looks like it might do what it says, and its upstreaming.
> 
> Acked-by: Andy Whitcroft <apw@canonical.com>
> 
> -apw

Patch

diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
index d2f143e..9986aba 100644
--- a/drivers/gpu/drm/nouveau/nv04_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
@@ -230,9 +230,9 @@  nv_crtc_mode_set_vga(struct drm_crtc *crtc, struct drm_display_mode *mode)
 	struct drm_framebuffer *fb = crtc->fb;
 
 	/* Calculate our timings */
-	int horizDisplay	= (mode->crtc_hdisplay >> 3) 	- 1;
-	int horizStart		= (mode->crtc_hsync_start >> 3) 	- 1;
-	int horizEnd		= (mode->crtc_hsync_end >> 3) 	- 1;
+	int horizDisplay	= (mode->crtc_hdisplay >> 3)		- 1;
+	int horizStart		= (mode->crtc_hsync_start >> 3) 	+ 1;
+	int horizEnd		= (mode->crtc_hsync_end >> 3)		+ 1;
 	int horizTotal		= (mode->crtc_htotal >> 3)		- 5;
 	int horizBlankStart	= (mode->crtc_hdisplay >> 3)		- 1;
 	int horizBlankEnd	= (mode->crtc_htotal >> 3)		- 1;