Em Fri, 26 Jun 2009 21:01:50 +0200 Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > Hi Mauro, > > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-dm646x for the following: > > - ARM: DaVinci: DM646x Video: VPIF driver > - ARM: DaVinci: DM646x Video: Add VPIF display driver Hmm... +static const struct vpif_channel_config_params ch_params[] = { + { + "NTSC", 720, 480, 30, 0, 1, 268, 1440, 1, 23, 263, 266, + 286, 525, 525, 0, 1, 0, V4L2_STD_525_60, + }, + { + "PAL", 720, 576, 25, 0, 1, 280, 1440, 1, 23, 311, 313, + 336, 624, 625, 0, 1, 0, V4L2_STD_625_50, + }, +}; This is a minor trouble, but the names for the standards above are wrong, since the output format has nothing to to with the way Chroma is encoded, but, instead, to the monochromatic standard. So, instead of NTSC, it should be STD/M (the name of the monochromatic standard used by the countries that has 60Hz line frequency, as defined at ITU-R recomendation BT.601 [1]). The STD/M standard is used by both PAL and NTSC color encodings. Also, instead of PAL, countries that has 50Hz line frequency use one of the non-STD/M monochromatic standards: STD/BDGHIKLN. Also, they can encode colors with either PAL or SECAM. [1] http://www.itu.int/rec/R-REC-BT.601/en +static int vpif_s_std(struct file *file, void *priv, v4l2_std_id *std_id) There's a small problem here: the userspace app could eventually send a request that could potentially match both a 50Hz and a 60Hz (for example, use V4L2_STD_ALL). In this case, you should be returning back the video standard by updating *std_id after selecting the standard: *std_id = <selected std id>; + for (j = 0; j < VPIF_DISPLAY_MAX_DEVICES; j++) { + ch = vpif_obj.dev[j]; + /* Initialize field of the channel objects */ + atomic_set(&ch->usrs, 0); + for (k = 0; k < VPIF_NUMOBJECTS; k++) { + ch->common[k].numbuffers = 0; + common = &ch->common[k]; + common->io_usrs = 0; + common->started = 0; + spin_lock_init(&common->irqlock); + mutex_init(&common->lock); + common->numbuffers = 0; + common->set_addr = NULL; + common->ytop_off = common->ybtm_off = 0; + common->ctop_off = common->cbtm_off = 0; + common->cur_frm = common->next_frm = NULL; + memset(&common->fmt, 0, sizeof(common->fmt)); + common->numbuffers = config_params.numbuffers[k]; + + } + ch->initialized = 0; Hmm... why are you needing to zero all those data structs? + /* Allocate memory for six channel objects */ + for (i = 0; i < VPIF_DISPLAY_MAX_DEVICES; i++) { + vpif_obj.dev[i] = + kmalloc(sizeof(struct channel_obj), GFP_KERNEL); Ah, that's the reason: you are using kmalloc, instead of kzalloc. Please, use kzalloc instead or use memset(&common, 0, sizeof(common)) instead. The code will be cleaner and probably faster. +#include <linux/version.h> Please move this from the .h to the .c file, since this is needed only at the places you have VIDIOC_QUERYCAP. > - ARM: DaVinci: DM646x Video: Makefile and config files modifications for Display > - ARM: DaVinci: DM646x Video: Fix compile time warnings for mutex locking The other patches are OK. Since all the above are minor adjustments, I'll pull from it and add at my -git. Please fix the above. [1] http://www.itu.int/rec/R-REC-BT.601/en Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
