Re: [RFC 2.6.26-rc9 1/5] pxafb: add shared framebuffer interface

["Jaya Kumar" <jayakumar.lkml@xxxxxxxxx>]

On Mon, Jul 21, 2008 at 7:11 AM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> On Sun, Jul 20, 2008 at 02:10:29AM -0400, Jaya Kumar wrote:
>>       if (fbi->map_cpu) {
>>               /* prevent initial garbage on screen */
>>               memset(fbi->map_cpu, 0, fbi->map_size);
>> -             fbi->fb.screen_base = fbi->map_cpu + fbi->video_offset;
>> +             fbi->fb.screen_base = (char __force __iomem *) fbi->map_cpu
>> +                                     + fbi->video_offset;
>
> I'm not sure this has any benefits - the use of __force in drivers is
> definitely frowned upon.
>
> It's well known that FB doesn't sit well with all the MMIO sparse checking
> so it's probably a good idea not to try and make it work.

Ok, I'll take it out. My intentions were to avoid causing any new
sparse warnings.

>
>> @@ -1357,6 +1379,8 @@ static void pxafb_decode_mode_info(struct pxafb_info *fbi,
>>               smemlen = modes[i].xres * modes[i].yres * modes[i].bpp / 8;
>>               if (smemlen > fbi->fb.fix.smem_len)
>>                       fbi->fb.fix.smem_len = smemlen;
>> +             if (inf->custom_xfer_div)
>> +                     fbi->fb.fix.smem_len /= inf->custom_xfer_div;
>
> I don't understand this.  If you have a 640x480 display with 8 bits of
> memory per pixel, then it's 307200 bytes no matter whether you're only
> using the lower 4 bits and the upper 4 are unused.
>
> If, on the other hand, the lower 4 bits contain one pixel and the upper
> 4 bits contain the other pixel, then you have a 4 bit per pixel
> framebuffer, so 'bpp' should be 4.  In which case the original calculation
> gives the correct answer.
>

The complexity I faced is that the transfers used by Metronome on the
AMLCD bus have to be 16-bits wide although it is actually treating the
pixels as 8-bits per pixel and then only using the lower 4-bits within
that for actual content. So what I did in the code is set bits per
pixel, bpp to 16 so that pxafb sets the LCCR3_16BPP flag (so that it
meets Metronome's AMLCD transfer requirements) and then use the
custom_xfer_div to divide the transfer sizes and buffer allocations to
the correct size.

If I set bpp to 8, the allocations and transfer sizes are correct but
then that causes pxafb to set LCCR3_8BPP and that causes Metronome to
fail to display anything.

Instead of doing custom_xfer_div, I had thought about setting
pxafb_mach_info.lccr3 = LCCR3_16BPP to try to force the setting of
LCCR3_16BPP but then I saw:

include/asm/arch-pxa/pxafb.h
         * The following should not be defined in LCCR3
         *      LCCR3_HSP, LCCR3_VSP, LCCR0_Pcd(x), LCCR3_Bpp
         */
        u_int           lccr3;

so I decided to create custom_xfer_div instead. How about if I try a
different way like creating a separate flag that can be used to
predicate the lccr3 setting. Maybe something like:

static int pxafb_bpp_to_lccr3(struct fb_var_screeninfo *var)
...
        case 8:
               switch (inf->custom_amlcd_bpp) {
                case 16: ret = LCCR3_16BPP; break;
                default: ret = LCCR3_8BPP; break;
        case 16: ret = LCCR3_16BPP; break;

Thanks,
jaya

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php