Re: [PATCH v2] cirrus-logic-framebuffer-i2c-support.patch

[Krzysztof Halasa <khc@pm.waw.pl>]

"Antonino A. Daplas" <adaplas@gmail.com> writes:

> David Eger is the author, the last I heard from him was 2 years ago.
> But I haven't received that many problem reports on cirrusfb.

Doesn't matter, what is important is that you know the stuff.

> The only register touched by the i2c code is EEPROM control (CL_SEQR8).
> This is never touched by the rest of the cirrusfb code. So I don't
> think concurrent access is a problem (unless the hardware has restrictions
> such as no other register accesses are allowed while this register is being
> accessed).

I mean I'm using (simplified):

unsigned char vga_rseq (void *regbase, unsigned char reg)
{
        vga_w (regbase, VGA_SEQ_I, reg);
        return vga_r (regbase, VGA_SEQ_D);
}

and

void vga_wseq (void *regbase, unsigned char reg, unsigned char val)
{
#ifdef VGA_OUTW_WRITE
        vga_w_fast (regbase, VGA_SEQ_I, reg, val);
#else
        vga_w (regbase, VGA_SEQ_I, reg);
        vga_w (regbase, VGA_SEQ_D, val);
#endif /* VGA_OUTW_WRITE */
}


How do I know the following will not happen:

taskA) alpine_getsda() called from I2C subsystem
 taskB) cirrusfb_blank() called from elsewhere
taskA) vga_rseq() calls vga_w()
 taskB) vga_rseq() or _wseq() calls vga_w()
taskA) vga_r() performed with wrong index (set by taskB).

I see most of vga_[rw]seq() calls are at init/set_mode time but still
this could be a problem.

This question isn't limited to cirrusfb only, of course.

> The framebuffer layer is serialized by
> acquire_console_sem()/release_console_sem(). If you think concurrent access
> is a problem, you can always use that.

It's quite big... While I haven't investigated that, I rather thought
about some small lock for vga_rseq() and vga_wseq(). Not sure.

Another thing... How is access to VGA registers shared between
X11 and the framebuffer?
-- 
Krzysztof Halasa