Re: [PATCH 1/1 2.6.24] fbdev: defio and Metronomefb v3

["Jaya Kumar" <jayakumar.lkml@gmail.com>]

On Sat, Feb 23, 2008 at 6:17 AM, Jaya Kumar <jayakumar.lkml@gmail.com> wrote:
> On Sat, Feb 23, 2008 at 3:07 AM, Andrew Morton
>  <akpm@linux-foundation.org> wrote:
>
> > On Mon, 18 Feb 2008 08:41:26 -0500 Jaya Kumar <jayakumar.lkml@gmail.com> wrote:
>  >
>  >
>  >
>  > > +static int metronome_display_cmd(struct metronomefb_par *par)
>  >  > +{
>  >  > +     int i;
>  >  > +     u16 cs;
>  >  > +     u16 opcode;
>  >  > +     static u8 borderval;
>  >  > +     u8 *ptr;
>  >  > +
>  >  > +     /* setup display command
>  >  > +     we can't immediately set the opcode since the controller
>  >  > +     will try parse the command before we've set it all up
>  >  > +     so we just set cs here and set the opcode at the end */
>  >  > +
>  >  > +     ptr = par->metromem;
>  >  > +
>  >  > +     if (par->metromem_cmd->opcode == 0xCC40)
>  >  > +             opcode = cs = 0xCC41;
>  >  > +     else
>  >  > +             opcode = cs = 0xCC40;
>  >  > +
>  >  > +     /* set the args ( 2 bytes ) for display */
>  >  > +     i = 0;
>  >  > +     par->metromem_cmd->args[i] =    1 << 3 /* border update */
>  >  > +                                     | ((borderval++ % 4) & 0x0F) << 4
>  >  > +                                     | (par->frame_count - 1) << 8;
>  >  > +     cs += par->metromem_cmd->args[i++];
>  >  > +
>  >  > +     /* the rest are 0 */
>  >  > +     memset((u8 *) (par->metromem_cmd->args + i), 0, (32-i)*2);
>  >  > +
>  >  > +     par->metromem_cmd->csum = cs;
>  >  > +     par->metromem_cmd->opcode = opcode; /* display cmd */
>  >  > +
>  >  > +     i = wait_event_interruptible_timeout(par->waitq, (GPLR1 & 0x01), HZ);
>  >  > +     return i;
>  >  > +}
>  >
>  >  wait_event_interruptible_timeout() will return immediately if the calling
>  >  task has a signal pending.  Will this ause the driver to malfunction?
>  >
>
>  Not sure yet. Will check.

The typical path for display_cmd is through the defio callback via the
mmap user. In this scenario, it is called from keventd rather than the
mmap user's context. Alternatively, display_cmd is called from the
write()/cfb path in the context of the user process. In either
scenario, a pending signal would cause us to return. The display
command would have been set telling the metronome controller to update
the display but the process would not be wait-ed. The implication of
this is that we could issue a new display cmd or do display work prior
to completion of the current display cmd. If we write to the
framebuffer during this time, the effect is that the checksum on the
transferred framebuffer data will not match and thus the controller
ignores the original display cmd, and will thus only process the last
display cmd which is the desired result anyway. Therefore, I think
this is okay.

The remaining *_cmd functions that use
wait_event_interruptible_timeout are all called from init_regs via
probe() so any signal results in the allocated resources being freed
by the failout in probe() and the error being returned which seems
okay. But after thinking about it, I'm changing these to use
wait_event_timeout since it doesn't make sense to be interruptible at
those junctures.

Thanks,
jaya

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/