Re: [patch] radeonfb: FB_WAITFORVSYNC implementation

["Ville Syrjälä" <syrjala@sci.fi>]

On Sat, Mar 12, 2005 at 02:06:45PM +0000, Torgeir Veimo wrote:
> This is an implementation of the FB_WAITFORVSYNC ioctl for the radeonfb.
> A small test application is attached at the end. This patch is against
> vanilla 2.6.11.

Some comements below.

Also adding support for FB_ACTIVATE_VBL should be quite simple now.

> --- linux-2.6.11-orig/drivers/video/aty/radeonfb.h	2005-03-02 07:38:37.000000000 +0000
> +++ linux-2.6.11/drivers/video/aty/radeonfb.h	2005-03-03 17:09:23.000000000 +0000
<snip>
> @@ -344,6 +349,11 @@
>  	struct timer_list	lvds_timer;
>  	u32			pending_lvds_gen_cntl;
>  
> +	int			open;
> +	struct radeon_interrupt	vblank;
> +	unsigned long		irq_flags;
> +	spinlock_t		int_lock;
> +

Unused spinlock.

>  #ifdef CONFIG_FB_RADEON_I2C
>  	struct radeon_i2c_chan 	i2c[4];
>  #endif
> --- linux-2.6.11-orig/drivers/video/aty/radeon_base.c	2005-03-02 07:37:54.000000000 +0000
> +++ linux-2.6.11/drivers/video/aty/radeon_base.c	2005-03-12 13:53:32.000000000 +0000
> @@ -68,6 +68,9 @@
>  #include <linux/ioport.h>
>  #include <linux/init.h>
>  #include <linux/pci.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>

Unused spinlock so no need for the header.

> +#include <linux/wait.h>
>  #include <linux/vmalloc.h>
>  #include <linux/device.h>
>  #include <linux/i2c.h>
> @@ -863,6 +866,97 @@
>  }
>  
>  
> +static irqreturn_t radeon_irq(int irq, void *dev_id, struct pt_regs *fp)
> +{
> +	struct radeonfb_info *rinfo = dev_id;
> +	u32 int_cntl = 0;
> +	u32 stat = 0;
> +
> +	int_cntl = INREG(GEN_INT_CNTL);
> +	stat = INREG(GEN_INT_STATUS) & (CRTC_VBLANK_MASK); 
> +	if (!stat)
> +		return IRQ_NONE;
> +	if (stat) {
> +		/* clear interrupt */
> +		OUTREG(GEN_INT_STATUS, stat);
> +
> +		(rinfo->vblank.count)++;

Nitpick: Drop the parentheses.

> +		wake_up_interruptible(&rinfo->vblank.wait);
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static int radeon_enable_irq(struct radeonfb_info *rinfo, int reenable)
> +{
> +	u32 int_cntl;
> +
> +	if (!test_and_set_bit(0, &rinfo->irq_flags)) {
> +		if (request_irq(rinfo->pdev->irq, radeon_irq, SA_SHIRQ, 
> +			"radeonfb", rinfo)) {
> +			printk("radeonfb: request_irq failed..\n");
> +			clear_bit(0, &rinfo->irq_flags);
> +			return -EINVAL;
> +		}
> +		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
> +		/* clear interrupt */
> +		//OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_STAT_ACK);

Why is this commented out?

> +		/* enable interrupt */
> +		OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_INT_EN);
> +		printk("radeonfb: enabled IRQ\n");
> +	} else if (reenable) {
> +		int_cntl = INREG(GEN_INT_CNTL) & CRTC_VBLANK_MASK;
> +		if (!(int_cntl & CRTC_VBLANK_INT_EN)) {
> +			printk("radeonfb: someone disabled IRQ [%08x]\n", int_cntl);
> +			/* re-enable interrupt */
> +			OUTREG(GEN_INT_CNTL, int_cntl | CRTC_VBLANK_INT_EN);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
<snip>
> @@ -2338,6 +2460,10 @@
>  	radeon_create_i2c_busses(rinfo);
>  #endif
>  
> +	rinfo->irq_flags = 0;

Nitpick: No need to set to 0. rinfo is memset() to 0 by 
framebuffer_alloc().

> +	init_waitqueue_head(&rinfo->vblank.wait);
> +	spin_lock_init(&(rinfo->int_lock));
> +

Unused spinlock.

>  	/* set all the vital stuff */
>  	radeon_set_fbinfo (rinfo);

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click