Re: [patch 09/11] ps3fb: Round up video modes

2008-01-27 Thread Geert Uytterhoeven
On Sat, 26 Jan 2008, Andrew Morton wrote:
> > On Fri, 25 Jan 2008 16:06:32 +0100 Geert Uytterhoeven <[EMAIL PROTECTED]> 
> > wrote:
> >  static int ps3fb_cmp_mode(const struct fb_videomode *vmode,
> >   const struct fb_var_screeninfo *var)
> >  {
> > -   /* resolution + black border must match a native resolution */
> > -   if (vmode->left_margin + vmode->xres + vmode->right_margin !=
> > -   var->left_margin + var->xres + var->right_margin ||
> > -   vmode->upper_margin + vmode->yres + vmode->lower_margin !=
> > -   var->upper_margin + var->yres + var->lower_margin)
> > +   long xres, yres, left_margin, right_margin, upper_margin, lower_margin;
> > +   long dx, dy;
> 
> I don't think these need to be longs?  And they probably don't need to be
> signed either.
> 
> If a switch to u32 improves the code any, it might be worth doing..
> 
> All the typecasting which this patch adds makes me wonder if the choice
> of types requires a general revisit...

The problems are:
  - Adding up the various fb_var_screeninfo.* timing fields (which are u32) may
cause overflows. That's why I use `long' (which is 64-bit on ppc64).
Perhaps I should change it to s64 for readability?
  - If I make xres, yres, left_margin, right_margin, upper_margin, lower_margin
u32, I need more casts for calculating dx and dy. Probably u64 would be OK,
though.
  - dx and dy are signed because they can be smaller than 0.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:+32 (0)2 700 8453
Fax:  +32 (0)2 700 8622
E-mail:   [EMAIL PROTECTED]
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [patch 09/11] ps3fb: Round up video modes

2008-01-26 Thread Andrew Morton
> On Fri, 25 Jan 2008 16:06:32 +0100 Geert Uytterhoeven <[EMAIL PROTECTED]> 
> wrote:
>  static int ps3fb_cmp_mode(const struct fb_videomode *vmode,
> const struct fb_var_screeninfo *var)
>  {
> - /* resolution + black border must match a native resolution */
> - if (vmode->left_margin + vmode->xres + vmode->right_margin !=
> - var->left_margin + var->xres + var->right_margin ||
> - vmode->upper_margin + vmode->yres + vmode->lower_margin !=
> - var->upper_margin + var->yres + var->lower_margin)
> + long xres, yres, left_margin, right_margin, upper_margin, lower_margin;
> + long dx, dy;

I don't think these need to be longs?  And they probably don't need to be
signed either.

If a switch to u32 improves the code any, it might be worth doing..

All the typecasting which this patch adds makes me wonder if the choice
of types requires a general revisit...
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[patch 09/11] ps3fb: Round up video modes

2008-01-25 Thread Geert Uytterhoeven
From: Geert Uytterhoeven <[EMAIL PROTECTED]>

ps3fb: Round up arbitrary video modes until they fit (if possible)

Signed-off-by: Geert Uytterhoeven <[EMAIL PROTECTED]>
---
 drivers/video/ps3fb.c |  160 +++---
 1 file changed, 114 insertions(+), 46 deletions(-)

--- a/drivers/video/ps3fb.c
+++ b/drivers/video/ps3fb.c
@@ -276,29 +276,49 @@ static char *mode_option __devinitdata;
 static int ps3fb_cmp_mode(const struct fb_videomode *vmode,
  const struct fb_var_screeninfo *var)
 {
-   /* resolution + black border must match a native resolution */
-   if (vmode->left_margin + vmode->xres + vmode->right_margin !=
-   var->left_margin + var->xres + var->right_margin ||
-   vmode->upper_margin + vmode->yres + vmode->lower_margin !=
-   var->upper_margin + var->yres + var->lower_margin)
+   long xres, yres, left_margin, right_margin, upper_margin, lower_margin;
+   long dx, dy;
+
+   /* maximum values */
+   if (var->xres > vmode->xres || var->yres > vmode->yres ||
+   var->pixclock > vmode->pixclock ||
+   var->hsync_len > vmode->hsync_len ||
+   var->vsync_len > vmode->vsync_len)
return -1;
 
-   /* minimum limits for margins */
-   if (vmode->left_margin > var->left_margin ||
-   vmode->right_margin > var->right_margin ||
-   vmode->upper_margin > var->upper_margin ||
-   vmode->lower_margin > var->lower_margin)
+   /* progressive/interlaced must match */
+   if ((var->vmode & FB_VMODE_MASK) != vmode->vmode)
return -1;
 
-   /* these fields must match exactly */
-   if (vmode->pixclock != var->pixclock ||
-   vmode->hsync_len != var->hsync_len ||
-   vmode->vsync_len != var->vsync_len ||
-   vmode->sync != var->sync ||
-   vmode->vmode != (var->vmode & FB_VMODE_MASK))
+   /* minimum resolution */
+   xres = max(var->xres, 1U);
+   yres = max(var->yres, 1U);
+
+   /* minimum margins */
+   left_margin = max(var->left_margin, vmode->left_margin);
+   right_margin = max(var->right_margin, vmode->right_margin);
+   upper_margin = max(var->upper_margin, vmode->upper_margin);
+   lower_margin = max(var->lower_margin, vmode->lower_margin);
+
+   /* resolution + margins may not exceed native parameters */
+   dx = ((long)vmode->left_margin + (long)vmode->xres +
+ (long)vmode->right_margin) -
+(left_margin + xres + right_margin);
+   if (dx < 0)
return -1;
 
-   return 0;
+   dy = ((long)vmode->upper_margin + (long)vmode->yres +
+ (long)vmode->lower_margin) -
+(upper_margin + yres + lower_margin);
+   if (dy < 0)
+   return -1;
+
+   /* exact match */
+   if (!dx && !dy)
+   return 0;
+
+   /* resolution difference */
+   return (vmode->xres - xres) * (vmode->yres - yres);
 }
 
 static const struct fb_videomode *ps3fb_native_vmode(enum ps3av_mode_num id)
@@ -324,33 +344,96 @@ static const struct fb_videomode *ps3fb_
 static unsigned int ps3fb_find_mode(struct fb_var_screeninfo *var,
u32 *ddr_line_length, u32 *xdr_line_length)
 {
-   unsigned int id;
+   unsigned int id, best_id;
+   int diff, best_diff;
const struct fb_videomode *vmode;
+   long gap;
 
+   best_id = 0;
+   best_diff = INT_MAX;
+   pr_debug("%s: wanted %u [%u] %u x %u [%u] %u\n", __func__,
+var->left_margin, var->xres, var->right_margin,
+var->upper_margin, var->yres, var->lower_margin);
for (id = PS3AV_MODE_480I; id <= PS3AV_MODE_WUXGA; id++) {
vmode = ps3fb_native_vmode(id);
-   if (!ps3fb_cmp_mode(vmode, var))
-   goto found;
+   diff = ps3fb_cmp_mode(vmode, var);
+   pr_debug("%s: mode %u: %u [%u] %u x %u [%u] %u: diff = %d\n",
+__func__, id, vmode->left_margin, vmode->xres,
+vmode->right_margin, vmode->upper_margin,
+vmode->yres, vmode->lower_margin, diff);
+   if (diff < 0)
+   continue;
+   if (diff < best_diff) {
+   best_id = id;
+   if (!diff)
+   break;
+   best_diff = diff;
+   }
}
 
-   pr_debug("%s: mode not found\n", __func__);
-   return 0;
+   if (!best_id) {
+   pr_debug("%s: no suitable mode found\n", __func__);
+   return 0;
+   }
+
+   id = best_id;
+   vmode = ps3fb_native_vmode(id);
 
-found:
*ddr_line_length = vmode->xres * BPP;
 
-   if (!var->xres) {
+   /* minimum resolution */
+   if (!var->xres)
var->xres = 1;
-   var->right_ma