linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91
@ 2008-08-08 12:46 Stanislaw Gruszka
  2008-08-08 16:44 ` Krzysztof Helt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2008-08-08 12:46 UTC (permalink / raw)
  To: Nicolas Ferre, linux-fbdev-devel

Panning in the y-direction can be done by simply changing the DMA base
address. This code is already in place, but FBIOPAN_DISPLAY will
currently fail because ypanstep is 0.

Set ypanstep to 1 to indicate that we do support y-panning and also
set the necessary acceleration flags on AT91 (AVR32 already have
them.)

Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
---
 drivers/video/atmel_lcdfb.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 5b3a15d..e1acfa7 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -39,7 +39,9 @@
 #endif
 
 #if defined(CONFIG_ARCH_AT91)
-#define	ATMEL_LCDFB_FBINFO_DEFAULT	FBINFO_DEFAULT
+#define	ATMEL_LCDFB_FBINFO_DEFAULT	(FBINFO_DEFAULT \
+					 | FBINFO_PARTIAL_PAN_OK \
+					 | FBINFO_HWACCEL_YPAN)
 
 static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
 					struct fb_var_screeninfo *var)
@@ -177,7 +179,7 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata 
= {
 	.type		= FB_TYPE_PACKED_PIXELS,
 	.visual		= FB_VISUAL_TRUECOLOR,
 	.xpanstep	= 0,
-	.ypanstep	= 0,
+	.ypanstep	= 1,
 	.ywrapstep	= 0,
 	.accel		= FB_ACCEL_NONE,
 };
-- 
1.5.2.5


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91
  2008-08-08 12:46 [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91 Stanislaw Gruszka
@ 2008-08-08 16:44 ` Krzysztof Helt
  2008-08-08 16:54 ` Krzysztof Helt
  2008-08-08 20:41 ` Andrew Morton
  2 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Helt @ 2008-08-08 16:44 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: linux-fbdev-devel

On Fri, 8 Aug 2008 14:46:43 +0200
Stanislaw Gruszka <stf_xl@wp.pl> wrote:

> Panning in the y-direction can be done by simply changing the DMA base
> address. This code is already in place, but FBIOPAN_DISPLAY will
> currently fail because ypanstep is 0.
> 
> Set ypanstep to 1 to indicate that we do support y-panning and also
> set the necessary acceleration flags on AT91 (AVR32 already have
> them.)
> 
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
> ---

Probably you both (Haavard and Stanislaw) should sign-off this patch.

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

>  drivers/video/atmel_lcdfb.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 5b3a15d..e1acfa7 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -39,7 +39,9 @@
>  #endif
>  
>  #if defined(CONFIG_ARCH_AT91)
> -#define	ATMEL_LCDFB_FBINFO_DEFAULT	FBINFO_DEFAULT
> +#define	ATMEL_LCDFB_FBINFO_DEFAULT	(FBINFO_DEFAULT \
> +					 | FBINFO_PARTIAL_PAN_OK \
> +					 | FBINFO_HWACCEL_YPAN)
>  
>  static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
>  					struct fb_var_screeninfo *var)
> @@ -177,7 +179,7 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata 
> = {
>  	.type		= FB_TYPE_PACKED_PIXELS,
>  	.visual		= FB_VISUAL_TRUECOLOR,
>  	.xpanstep	= 0,
> -	.ypanstep	= 0,
> +	.ypanstep	= 1,
>  	.ywrapstep	= 0,
>  	.accel		= FB_ACCEL_NONE,
>  };
> -- 
> 1.5.2.5
> 
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 


----------------------------------------------------------------------
Tylko dla detektywow! Konkurs na Smaker.pl
Kliknij >>> http://link.interia.pl/f1eb1


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91
  2008-08-08 12:46 [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91 Stanislaw Gruszka
  2008-08-08 16:44 ` Krzysztof Helt
@ 2008-08-08 16:54 ` Krzysztof Helt
  2008-08-08 20:41 ` Andrew Morton
  2 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Helt @ 2008-08-08 16:54 UTC (permalink / raw)
  To: Stanislaw Gruszka, Andrew Morton; +Cc: linux-fbdev-devel

On Fri, 8 Aug 2008 14:46:43 +0200
Stanislaw Gruszka <stf_xl@wp.pl> wrote:

> Panning in the y-direction can be done by simply changing the DMA base
> address. This code is already in place, but FBIOPAN_DISPLAY will
> currently fail because ypanstep is 0.
> 
> Set ypanstep to 1 to indicate that we do support y-panning and also
> set the necessary acceleration flags on AT91 (AVR32 already have
> them.)
> 
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
> ---

Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>

Once again, now with Andrew Morton included.

Regards,
Krzysztof

>  drivers/video/atmel_lcdfb.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 5b3a15d..e1acfa7 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -39,7 +39,9 @@
>  #endif
>  
>  #if defined(CONFIG_ARCH_AT91)
> -#define	ATMEL_LCDFB_FBINFO_DEFAULT	FBINFO_DEFAULT
> +#define	ATMEL_LCDFB_FBINFO_DEFAULT	(FBINFO_DEFAULT \
> +					 | FBINFO_PARTIAL_PAN_OK \
> +					 | FBINFO_HWACCEL_YPAN)
>  
>  static inline void atmel_lcdfb_update_dma2d(struct atmel_lcdfb_info *sinfo,
>  					struct fb_var_screeninfo *var)
> @@ -177,7 +179,7 @@ static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata 
> = {
>  	.type		= FB_TYPE_PACKED_PIXELS,
>  	.visual		= FB_VISUAL_TRUECOLOR,
>  	.xpanstep	= 0,
> -	.ypanstep	= 0,
> +	.ypanstep	= 1,
>  	.ywrapstep	= 0,
>  	.accel		= FB_ACCEL_NONE,
>  };
> -- 
> 1.5.2.5
> 
> 
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel
> 


----------------------------------------------------------------------
Tylko dla detektywow! Konkurs na Smaker.pl
Kliknij >>> http://link.interia.pl/f1eb1


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91
  2008-08-08 12:46 [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91 Stanislaw Gruszka
  2008-08-08 16:44 ` Krzysztof Helt
  2008-08-08 16:54 ` Krzysztof Helt
@ 2008-08-08 20:41 ` Andrew Morton
  2008-08-09 13:57   ` Stanislaw Gruszka
  2008-08-09 14:41   ` Haavard Skinnemoen
  2 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2008-08-08 20:41 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: linux-fbdev-devel

On Fri, 8 Aug 2008 14:46:43 +0200
Stanislaw Gruszka <stf_xl@wp.pl> wrote:

> Panning in the y-direction can be done by simply changing the DMA base
> address. This code is already in place, but FBIOPAN_DISPLAY will
> currently fail because ypanstep is 0.
> 
> Set ypanstep to 1 to indicate that we do support y-panning and also
> set the necessary acceleration flags on AT91 (AVR32 already have
> them.)
> 
> Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>

problems...

a) Neither of these changelogs communicate the seriousness of the
   problem which is being fixed, nor the benefit of the change.

   So when I go through my usual "do we need this in 2.6.27? 
   2.6.26?  2.6.25?" exercise, I don't have enough information to be
   able to tell.

b) The second patch was wordwrapped by your email client.  I fixed that.

c) The authorship/signoff is confusing.  The patch is From:you and
   signed-off-by:Haavard but is missing your signed-off-by:.

   Who wrote the patches?  If it was you then all we're missing is
   your signed-off-by:.

   If it was Haavard then you should have had his From: line at the
   start of the chagnelog to indicate this.

   Either way, it should have had your signed-off-by:, because you
   were in the delivery path.

   If Haavard did not participate in the development and was not in
   the delivery path then his signed-off-by: was inappropriate, and an
   acked-by: or tested-by: or reviewed-by: would be better.


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91
  2008-08-08 20:41 ` Andrew Morton
@ 2008-08-09 13:57   ` Stanislaw Gruszka
  2008-08-09 14:41   ` Haavard Skinnemoen
  1 sibling, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2008-08-09 13:57 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-fbdev-devel

On Friday 08 August 2008, Andrew Morton wrote:
> On Fri, 8 Aug 2008 14:46:43 +0200
>
> Stanislaw Gruszka <stf_xl@wp.pl> wrote:
> > Panning in the y-direction can be done by simply changing the DMA base
> > address. This code is already in place, but FBIOPAN_DISPLAY will
> > currently fail because ypanstep is 0.
> >
> > Set ypanstep to 1 to indicate that we do support y-panning and also
> > set the necessary acceleration flags on AT91 (AVR32 already have
> > them.)
> >
> > Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
>
> problems...
>
> a) Neither of these changelogs communicate the seriousness of the
>    problem which is being fixed, nor the benefit of the change.
>
>    So when I go through my usual "do we need this in 2.6.27?
>    2.6.26?  2.6.25?" exercise, I don't have enough information to be
>    able to tell.
I think this is a new feature, should go to future release i.e. 2.6.27.

> b) The second patch was wordwrapped by your email client.  I fixed that.
I sent a patch average ones a year and never did it correctly, maybe if I will 
be sending patches more frequently, I will learn do it good ;-)

> c) The authorship/signoff is confusing.  The patch is From:you and
>    signed-off-by:Haavard but is missing your signed-off-by:.
>
>    Who wrote the patches?  If it was you then all we're missing is
>    your signed-off-by:.
>
>    If it was Haavard then you should have had his From: line at the
>    start of the chagnelog to indicate this.
Patch is from Haavard, sorry for removing From:

>    Either way, it should have had your signed-off-by:, because you
>    were in the delivery path.
Sorry again, to late but ...
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91
  2008-08-08 20:41 ` Andrew Morton
  2008-08-09 13:57   ` Stanislaw Gruszka
@ 2008-08-09 14:41   ` Haavard Skinnemoen
  1 sibling, 0 replies; 6+ messages in thread
From: Haavard Skinnemoen @ 2008-08-09 14:41 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-fbdev-devel

Andrew Morton <akpm@linux-foundation.org> wrote:
> a) Neither of these changelogs communicate the seriousness of the
>    problem which is being fixed, nor the benefit of the change.

The benefit is that the FBIOPAN_DISPLAY ioctl can now be used to pan the
display in the y direction on avr32 and at91. The seriousness is
probably not very bad since I'm sure there's hardware that just doesn't
support this kind of thing at all. But it is necessary for double- and
triple-buffering to work.

The existing patch description does say this (FBIOPAN_DISPLAY doesn't
work since ypanstep is 0, so we set it to 1), though it doesn't explain
what FBIOPAN_DISPLAY does and how serious it is that it isn't working. I
guess I took it for granted that the fbdev people knew this already ;-)

Some chips support x-panning too, so we should probably have a look and
see if that's set up correctly. But that's less important IMO.

>    So when I go through my usual "do we need this in 2.6.27? 
>    2.6.26?  2.6.25?" exercise, I don't have enough information to be
>    able to tell.

I'd say 2.6.27. I've had the patch applied to my "vendor tree" for
about a year, so it's well-tested on avr32, and Stanislaw confirms it
also works on at91. In other words, the risk is not very high, but
since it's a new feature it probably doesn't deserve to be backported.

> b) The second patch was wordwrapped by your email client.  I fixed that.
> 
> c) The authorship/signoff is confusing.  The patch is From:you and
>    signed-off-by:Haavard but is missing your signed-off-by:.
> 
>    Who wrote the patches?  If it was you then all we're missing is
>    your signed-off-by:.
> 
>    If it was Haavard then you should have had his From: line at the
>    start of the chagnelog to indicate this.
> 
>    Either way, it should have had your signed-off-by:, because you
>    were in the delivery path.
> 
>    If Haavard did not participate in the development and was not in
>    the delivery path then his signed-off-by: was inappropriate, and an
>    acked-by: or tested-by: or reviewed-by: would be better.

I wrote the patch, so it's missing from:me and signed-off-by:Stanislaw.

Haavard

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-08-09 14:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-08 12:46 [PATCH 1/2] atmel_lcdfb: Set ypanstep to 1 and enable y-panning on AT91 Stanislaw Gruszka
2008-08-08 16:44 ` Krzysztof Helt
2008-08-08 16:54 ` Krzysztof Helt
2008-08-08 20:41 ` Andrew Morton
2008-08-09 13:57   ` Stanislaw Gruszka
2008-08-09 14:41   ` Haavard Skinnemoen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).