>Ben Dooks <mailto:ben@xxxxxxxxxxxx> wrote: >On 28/06/10 09:08, Pawel Osciak wrote: >> @@ -801,6 +825,124 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var, >> return 0; >> } >> >> +/** >> + * s3c_fb_enable_irq() - enable framebuffer interrupts >> + * @sfb: main hardware state >> + */ >> +static void s3c_fb_enable_irq(struct s3c_fb *sfb) >> +{ >> + void __iomem *regs = sfb->regs; >> + u32 irq_ctrl_reg; >> + >> + if (!test_and_set_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) { >> + /* IRQ disabled, enable it */ >> + irq_ctrl_reg = readl(regs + VIDINTCON0); >> + >> + irq_ctrl_reg |= VIDINTCON0_INT_ENABLE; >> + irq_ctrl_reg |= VIDINTCON0_INT_FRAME; >> + >> + irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL0_MASK; >> + irq_ctrl_reg |= VIDINTCON0_FRAMESEL0_VSYNC; >> + irq_ctrl_reg &= ~VIDINTCON0_FRAMESEL1_MASK; >> + irq_ctrl_reg |= VIDINTCON0_FRAMESEL1_NONE; >> + >> + writel(irq_ctrl_reg, regs + VIDINTCON0); >> + } >> +} > >there should probably be some form of locking so that an irq >doesn't come through and disable this. possibly in the call >that queues the request to reduce the likelyhood of any races. > Swapping the count and enable_irq below will fix this. >> + >> +/** >> + * s3c_fb_disable_irq() - disable framebuffer interrupts >> + * @sfb: main hardware state >> + */ >> +static void s3c_fb_disable_irq(struct s3c_fb *sfb) >> +{ >> + void __iomem *regs = sfb->regs; >> + u32 irq_ctrl_reg; >> + >> + if (test_and_clear_bit(S3C_FB_VSYNC_IRQ_EN, &sfb->irq_flags)) { >> + /* IRQ enabled, disable it */ >> + irq_ctrl_reg = readl(regs + VIDINTCON0); >> + >> + irq_ctrl_reg &= ~VIDINTCON0_INT_FRAME; >> + irq_ctrl_reg &= ~VIDINTCON0_INT_ENABLE; >> + >> + writel(irq_ctrl_reg, regs + VIDINTCON0); >> + } >> +} >> + >> +static irqreturn_t s3c_fb_irq(int irq, void *dev_id) >> +{ >> + struct s3c_fb *sfb = dev_id; >> + void __iomem *regs = sfb->regs; >> + u32 irq_sts_reg; >> + >> + irq_sts_reg = readl(regs + VIDINTCON1); >> + >> + if (irq_sts_reg & VIDINTCON1_INT_FRAME) { >> + >> + /* VSYNC interrupt, accept it */ >> + writel(VIDINTCON1_INT_FRAME, regs + VIDINTCON1); >> + >> + sfb->vsync_info.count++; >> + wake_up_interruptible(&sfb->vsync_info.wait); >> + } >> + >> + /* We only support waiting for VSYNC for now, so it's safe >> + * to always disable irqs here. >> + */ >> + s3c_fb_disable_irq(sfb); >> + >> + return IRQ_HANDLED; >> +} >> + >> +/** >> + * s3c_fb_wait_for_vsync() - sleep until next VSYNC interrupt or timeout >> + * @sfb: main hardware state >> + * @crtc: head index. >> + */ >> +static int s3c_fb_wait_for_vsync(struct s3c_fb *sfb, u32 crtc) >> +{ >> + unsigned long count; >> + int ret; >> + >> + if (crtc != 0) >> + return -ENODEV; >> + >> + s3c_fb_enable_irq(sfb); >> + count = sfb->vsync_info.count; > >possibly doing count before enabling the irq. > yes... can't get my head around to what I've been thinking when writing this, it's been a year ago... I remember being quite sure that it'd work that way then though, as ridiculous as it looks right now... Will swap them. >> + ret = wait_event_interruptible_timeout(sfb->vsync_info.wait, >> + count != sfb->vsync_info.count, >> + msecs_to_jiffies(VSYNC_TIMEOUT_MSEC)); >> + if (ret == 0) >> + return -ETIMEDOUT; >> + >> + return 0; >> +} >> + >> +static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd, >> + unsigned long arg) >> +{ >> + struct s3c_fb_win *win = info->par; >> + struct s3c_fb *sfb = win->parent; >> + int ret; >> + u32 crtc; >> + >> + switch (cmd) { >> + case FBIO_WAITFORVSYNC: >> + if (get_user(crtc, (u32 __user *)arg)) { >> + ret = -EFAULT; >> + break; >> + } > >can't find any info on what the argument is meant to be. > Head number, i.e. crt no I believe. Not sure though. >> + >> + ret = s3c_fb_wait_for_vsync(sfb, crtc); >> + break; >> + default: >> + ret = -ENOTTY; > >Can someone else confirm this is the correct err? > Linux Device Drivers 3, page 140, or see the online version here: http://www.makelinux.net/ldd3/chp-6-sect-1.shtml section 6.1.2. The Return Value man ioctl: ENOTTY The specified request does not apply to the kind of object that the descriptor d references. >> @@ -1370,6 +1534,7 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 >__devinitdata = { >> [3] = 0x3000, >> [4] = 0x3400, >> }, >> + >> }, > >oops, added whitespace. > will fix. Best regards -- Pawel Osciak Linux Platform Group Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |