[media] omap3isp: preview: Shorten shadow update delay
When applications modify preview engine parameters, the new values are
applied to the hardware by the preview engine interrupt handler during
vertical blanking. If the parameters are being changed when the
interrupt handler is called, it just delays applying the parameters
until the next frame.
If an application modifies the parameters for every frame, and the
preview engine interrupt is triggerred synchronously, the parameters are
never applied to the hardware.
Fix this by storing new parameters in a shadow copy, and switch the
active parameters with the shadow values atomically.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
diff --git a/drivers/media/video/omap3isp/isppreview.c b/drivers/media/video/omap3isp/isppreview.c
index fd1807d..5bdf9e7 100644
--- a/drivers/media/video/omap3isp/isppreview.c
+++ b/drivers/media/video/omap3isp/isppreview.c
@@ -649,12 +649,18 @@
static void
preview_update_contrast(struct isp_prev_device *prev, u8 contrast)
{
- struct prev_params *params = &prev->params;
+ struct prev_params *params;
+ unsigned long flags;
+
+ spin_lock_irqsave(&prev->params.lock, flags);
+ params = (prev->params.active & OMAP3ISP_PREV_CONTRAST)
+ ? &prev->params.params[0] : &prev->params.params[1];
if (params->contrast != (contrast * ISPPRV_CONTRAST_UNITS)) {
params->contrast = contrast * ISPPRV_CONTRAST_UNITS;
- prev->update |= OMAP3ISP_PREV_CONTRAST;
+ params->update |= OMAP3ISP_PREV_CONTRAST;
}
+ spin_unlock_irqrestore(&prev->params.lock, flags);
}
/*
@@ -681,12 +687,18 @@
static void
preview_update_brightness(struct isp_prev_device *prev, u8 brightness)
{
- struct prev_params *params = &prev->params;
+ struct prev_params *params;
+ unsigned long flags;
+
+ spin_lock_irqsave(&prev->params.lock, flags);
+ params = (prev->params.active & OMAP3ISP_PREV_BRIGHTNESS)
+ ? &prev->params.params[0] : &prev->params.params[1];
if (params->brightness != (brightness * ISPPRV_BRIGHT_UNITS)) {
params->brightness = brightness * ISPPRV_BRIGHT_UNITS;
- prev->update |= OMAP3ISP_PREV_BRIGHTNESS;
+ params->update |= OMAP3ISP_PREV_BRIGHTNESS;
}
+ spin_unlock_irqrestore(&prev->params.lock, flags);
}
/*
@@ -721,6 +733,75 @@
OMAP3_ISP_IOMEM_PREV, ISPPRV_SETUP_YC);
}
+static u32
+preview_params_lock(struct isp_prev_device *prev, u32 update, bool shadow)
+{
+ u32 active = prev->params.active;
+
+ if (shadow) {
+ /* Mark all shadow parameters we are going to touch as busy. */
+ prev->params.params[0].busy |= ~active & update;
+ prev->params.params[1].busy |= active & update;
+ } else {
+ /* Mark all active parameters we are going to touch as busy. */
+ update = (prev->params.params[0].update & active)
+ | (prev->params.params[1].update & ~active);
+
+ prev->params.params[0].busy |= active & update;
+ prev->params.params[1].busy |= ~active & update;
+ }
+
+ return update;
+}
+
+static void
+preview_params_unlock(struct isp_prev_device *prev, u32 update, bool shadow)
+{
+ u32 active = prev->params.active;
+
+ if (shadow) {
+ /* Set the update flag for shadow parameters that have been
+ * updated and clear the busy flag for all shadow parameters.
+ */
+ prev->params.params[0].update |= (~active & update);
+ prev->params.params[1].update |= (active & update);
+ prev->params.params[0].busy &= active;
+ prev->params.params[1].busy &= ~active;
+ } else {
+ /* Clear the update flag for active parameters that have been
+ * applied and the busy flag for all active parameters.
+ */
+ prev->params.params[0].update &= ~(active & update);
+ prev->params.params[1].update &= ~(~active & update);
+ prev->params.params[0].busy &= ~active;
+ prev->params.params[1].busy &= active;
+ }
+}
+
+static void preview_params_switch(struct isp_prev_device *prev)
+{
+ u32 to_switch;
+
+ /* Switch active parameters with updated shadow parameters when the
+ * shadow parameter has been updated and neither the active not the
+ * shadow parameter is busy.
+ */
+ to_switch = (prev->params.params[0].update & ~prev->params.active)
+ | (prev->params.params[1].update & prev->params.active);
+ to_switch &= ~(prev->params.params[0].busy |
+ prev->params.params[1].busy);
+ if (to_switch == 0)
+ return;
+
+ prev->params.active ^= to_switch;
+
+ /* Remove the update flag for the shadow copy of parameters we have
+ * switched.
+ */
+ prev->params.params[0].update &= ~(~prev->params.active & to_switch);
+ prev->params.params[1].update &= ~(prev->params.active & to_switch);
+}
+
/* preview parameters update structure */
struct preview_update {
void (*config)(struct isp_prev_device *, const void *);
@@ -834,37 +915,6 @@
};
/*
- * __preview_get_ptrs - helper function which return pointers to members
- * of params and config structures.
- * @params - pointer to preview_params structure.
- * @param - return pointer to appropriate structure field.
- * @configs - pointer to update config structure.
- * @config - return pointer to appropriate structure field.
- * @bit - for which feature to return pointers.
- * Return size of corresponding prev_params member
- */
-static u32
-__preview_get_ptrs(struct prev_params *params, void **param,
- struct omap3isp_prev_update_config *configs,
- void __user **config, unsigned int index)
-{
- const struct preview_update *info = &update_attrs[index];
-
- if (index >= ARRAY_SIZE(update_attrs)) {
- if (config)
- *config = NULL;
- *param = NULL;
- return 0;
- }
-
- if (configs && config)
- *config = *(void **)((void *)configs + info->config_offset);
-
- *param = (void *)params + info->param_offset;
- return info->param_size;
-}
-
-/*
* preview_config - Copy and update local structure with userspace preview
* configuration.
* @prev: ISP preview engine
@@ -876,37 +926,41 @@
static int preview_config(struct isp_prev_device *prev,
struct omap3isp_prev_update_config *cfg)
{
- const struct preview_update *attr;
- struct prev_params *params;
- int i, bit, rval = 0;
+ unsigned long flags;
+ unsigned int i;
+ int rval = 0;
+ u32 update;
+ u32 active;
if (cfg->update == 0)
return 0;
- params = &prev->params;
+ /* Mark the shadow parameters we're going to update as busy. */
+ spin_lock_irqsave(&prev->params.lock, flags);
+ preview_params_lock(prev, cfg->update, true);
+ active = prev->params.active;
+ spin_unlock_irqrestore(&prev->params.lock, flags);
- if (prev->state != ISP_PIPELINE_STREAM_STOPPED) {
- unsigned long flags;
-
- spin_lock_irqsave(&prev->lock, flags);
- prev->shadow_update = 1;
- spin_unlock_irqrestore(&prev->lock, flags);
- }
+ update = 0;
for (i = 0; i < ARRAY_SIZE(update_attrs); i++) {
- attr = &update_attrs[i];
- bit = 1 << i;
+ const struct preview_update *attr = &update_attrs[i];
+ struct prev_params *params;
+ unsigned int bit = 1 << i;
if (attr->skip || !(cfg->update & bit))
continue;
- if (cfg->flag & bit) {
- void *to = NULL, __user *from = NULL;
- unsigned long sz = 0;
+ params = &prev->params.params[!!(active & bit)];
- sz = __preview_get_ptrs(params, &to, cfg, &from, i);
- if (to && from && sz) {
- if (copy_from_user(to, from, sz)) {
+ if (cfg->flag & bit) {
+ void __user *from = *(void * __user *)
+ ((void *)cfg + attr->config_offset);
+ void *to = (void *)params + attr->param_offset;
+ size_t size = attr->param_size;
+
+ if (to && from && size) {
+ if (copy_from_user(to, from, size)) {
rval = -EFAULT;
break;
}
@@ -916,50 +970,59 @@
params->features &= ~bit;
}
- prev->update |= bit;
+ update |= bit;
}
- prev->shadow_update = 0;
+ spin_lock_irqsave(&prev->params.lock, flags);
+ preview_params_unlock(prev, update, true);
+ preview_params_switch(prev);
+ spin_unlock_irqrestore(&prev->params.lock, flags);
+
return rval;
}
/*
* preview_setup_hw - Setup preview registers and/or internal memory
* @prev: pointer to preview private structure
+ * @update: Bitmask of parameters to setup
+ * @active: Bitmask of parameters active in set 0
* Note: can be called from interrupt context
* Return none
*/
-static void preview_setup_hw(struct isp_prev_device *prev)
+static void preview_setup_hw(struct isp_prev_device *prev, u32 update,
+ u32 active)
{
- struct prev_params *params = &prev->params;
- const struct preview_update *attr;
- unsigned int bit;
unsigned int i;
- void *param_ptr;
+ u32 features;
- if (prev->update == 0)
+ if (update == 0)
return;
- for (i = 0; i < ARRAY_SIZE(update_attrs); i++) {
- attr = &update_attrs[i];
- bit = 1 << i;
+ features = (prev->params.params[0].features & active)
+ | (prev->params.params[1].features & ~active);
- if (!(prev->update & bit))
+ for (i = 0; i < ARRAY_SIZE(update_attrs); i++) {
+ const struct preview_update *attr = &update_attrs[i];
+ struct prev_params *params;
+ unsigned int bit = 1 << i;
+ void *param_ptr;
+
+ if (!(update & bit))
continue;
+ params = &prev->params.params[!(active & bit)];
+
if (params->features & bit) {
if (attr->config) {
- __preview_get_ptrs(params, ¶m_ptr, NULL,
- NULL, i);
+ param_ptr = (void *)params + attr->param_offset;
attr->config(prev, param_ptr);
}
if (attr->enable)
attr->enable(prev, 1);
- } else
+ } else {
if (attr->enable)
attr->enable(prev, 0);
-
- prev->update &= ~bit;
+ }
}
}
@@ -997,13 +1060,17 @@
static void preview_config_averager(struct isp_prev_device *prev, u8 average)
{
struct isp_device *isp = to_isp_device(prev);
+ struct prev_params *params;
int reg = 0;
- if (prev->params.cfa.format == OMAP3ISP_CFAFMT_BAYER)
+ params = (prev->params.active & OMAP3ISP_PREV_CFA)
+ ? &prev->params.params[0] : &prev->params.params[1];
+
+ if (params->cfa.format == OMAP3ISP_CFAFMT_BAYER)
reg = ISPPRV_AVE_EVENDIST_2 << ISPPRV_AVE_EVENDIST_SHIFT |
ISPPRV_AVE_ODDDIST_2 << ISPPRV_AVE_ODDDIST_SHIFT |
average;
- else if (prev->params.cfa.format == OMAP3ISP_CFAFMT_RGBFOVEON)
+ else if (params->cfa.format == OMAP3ISP_CFAFMT_RGBFOVEON)
reg = ISPPRV_AVE_EVENDIST_3 << ISPPRV_AVE_EVENDIST_SHIFT |
ISPPRV_AVE_ODDDIST_3 << ISPPRV_AVE_ODDDIST_SHIFT |
average;
@@ -1021,33 +1088,35 @@
*
* See the explanation at the PREV_MARGIN_* definitions for more details.
*/
-static void preview_config_input_size(struct isp_prev_device *prev)
+static void preview_config_input_size(struct isp_prev_device *prev, u32 active)
{
struct isp_device *isp = to_isp_device(prev);
- struct prev_params *params = &prev->params;
unsigned int sph = prev->crop.left;
unsigned int eph = prev->crop.left + prev->crop.width - 1;
unsigned int slv = prev->crop.top;
unsigned int elv = prev->crop.top + prev->crop.height - 1;
+ u32 features;
- if (params->features & OMAP3ISP_PREV_CFA) {
+ features = (prev->params.params[0].features & active)
+ | (prev->params.params[1].features & ~active);
+
+ if (features & OMAP3ISP_PREV_CFA) {
sph -= 2;
eph += 2;
slv -= 2;
elv += 2;
}
- if (params->features & (OMAP3ISP_PREV_DEFECT_COR | OMAP3ISP_PREV_NF)) {
+ if (features & (OMAP3ISP_PREV_DEFECT_COR | OMAP3ISP_PREV_NF)) {
sph -= 2;
eph += 2;
slv -= 2;
elv += 2;
}
- if (params->features & OMAP3ISP_PREV_HRZ_MED) {
+ if (features & OMAP3ISP_PREV_HRZ_MED) {
sph -= 2;
eph += 2;
}
- if (params->features & (OMAP3ISP_PREV_CHROMA_SUPP |
- OMAP3ISP_PREV_LUMAENH))
+ if (features & (OMAP3ISP_PREV_CHROMA_SUPP | OMAP3ISP_PREV_LUMAENH))
sph -= 2;
isp_reg_writel(isp, (sph << ISPPRV_HORZ_INFO_SPH_SHIFT) | eph,
@@ -1182,8 +1251,16 @@
*/
void omap3isp_preview_restore_context(struct isp_device *isp)
{
- isp->isp_prev.update = OMAP3ISP_PREV_FEATURES_END - 1;
- preview_setup_hw(&isp->isp_prev);
+ struct isp_prev_device *prev = &isp->isp_prev;
+ const u32 update = OMAP3ISP_PREV_FEATURES_END - 1;
+
+ prev->params.params[0].update = prev->params.active & update;
+ prev->params.params[1].update = ~prev->params.active & update;
+
+ preview_setup_hw(prev, update, prev->params.active);
+
+ prev->params.params[0].update = 0;
+ prev->params.params[1].update = 0;
}
/*
@@ -1242,12 +1319,21 @@
/*
* preview_init_params - init image processing parameters.
* @prev: pointer to previewer private structure
- * return none
*/
static void preview_init_params(struct isp_prev_device *prev)
{
- struct prev_params *params = &prev->params;
- int i = 0;
+ struct prev_params *params;
+ unsigned int i;
+
+ spin_lock_init(&prev->params.lock);
+
+ prev->params.active = ~0;
+ prev->params.params[0].busy = 0;
+ prev->params.params[0].update = OMAP3ISP_PREV_FEATURES_END - 1;
+ prev->params.params[1].busy = 0;
+ prev->params.params[1].update = 0;
+
+ params = &prev->params.params[0];
/* Init values */
params->contrast = ISPPRV_CONTRAST_DEF * ISPPRV_CONTRAST_UNITS;
@@ -1291,8 +1377,6 @@
| OMAP3ISP_PREV_RGB2RGB | OMAP3ISP_PREV_COLOR_CONV
| OMAP3ISP_PREV_WB | OMAP3ISP_PREV_BRIGHTNESS
| OMAP3ISP_PREV_CONTRAST;
-
- prev->update = OMAP3ISP_PREV_FEATURES_END - 1;
}
/*
@@ -1321,8 +1405,17 @@
{
struct isp_device *isp = to_isp_device(prev);
struct v4l2_mbus_framefmt *format;
+ unsigned long flags;
+ u32 update;
+ u32 active;
- preview_setup_hw(prev);
+ spin_lock_irqsave(&prev->params.lock, flags);
+ /* Mark all active parameters we are going to touch as busy. */
+ update = preview_params_lock(prev, 0, false);
+ active = prev->params.active;
+ spin_unlock_irqrestore(&prev->params.lock, flags);
+
+ preview_setup_hw(prev, update, active);
if (prev->output & PREVIEW_OUTPUT_MEMORY)
isp_reg_set(isp, OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR,
@@ -1343,7 +1436,7 @@
preview_adjust_bandwidth(prev);
- preview_config_input_size(prev);
+ preview_config_input_size(prev, active);
if (prev->input == PREVIEW_INPUT_CCDC)
preview_config_inlineoffset(prev, 0);
@@ -1360,6 +1453,10 @@
preview_config_averager(prev, 0);
preview_config_ycpos(prev, format->code);
+
+ spin_lock_irqsave(&prev->params.lock, flags);
+ preview_params_unlock(prev, update, false);
+ spin_unlock_irqrestore(&prev->params.lock, flags);
}
/* -----------------------------------------------------------------------------
@@ -1448,25 +1545,30 @@
void omap3isp_preview_isr(struct isp_prev_device *prev)
{
unsigned long flags;
+ u32 update;
+ u32 active;
if (omap3isp_module_sync_is_stopping(&prev->wait, &prev->stopping))
return;
- spin_lock_irqsave(&prev->lock, flags);
- if (prev->shadow_update)
- goto done;
+ spin_lock_irqsave(&prev->params.lock, flags);
+ preview_params_switch(prev);
+ update = preview_params_lock(prev, 0, false);
+ active = prev->params.active;
+ spin_unlock_irqrestore(&prev->params.lock, flags);
- preview_setup_hw(prev);
- preview_config_input_size(prev);
-
-done:
- spin_unlock_irqrestore(&prev->lock, flags);
+ preview_setup_hw(prev, update, active);
+ preview_config_input_size(prev, active);
if (prev->input == PREVIEW_INPUT_MEMORY ||
prev->output & PREVIEW_OUTPUT_MEMORY)
preview_isr_buffer(prev);
else if (prev->state == ISP_PIPELINE_STREAM_CONTINUOUS)
preview_enable_oneshot(prev);
+
+ spin_lock_irqsave(&prev->params.lock, flags);
+ preview_params_unlock(prev, update, false);
+ spin_unlock_irqrestore(&prev->params.lock, flags);
}
/* -----------------------------------------------------------------------------
@@ -1552,7 +1654,6 @@
struct isp_video *video_out = &prev->video_out;
struct isp_device *isp = to_isp_device(prev);
struct device *dev = to_device(prev);
- unsigned long flags;
if (prev->state == ISP_PIPELINE_STREAM_STOPPED) {
if (enable == ISP_PIPELINE_STREAM_STOPPED)
@@ -1589,11 +1690,9 @@
if (omap3isp_module_sync_idle(&sd->entity, &prev->wait,
&prev->stopping))
dev_dbg(dev, "%s: stop timeout.\n", sd->name);
- spin_lock_irqsave(&prev->lock, flags);
omap3isp_sbl_disable(isp, OMAP3_ISP_SBL_PREVIEW_READ);
omap3isp_sbl_disable(isp, OMAP3_ISP_SBL_PREVIEW_WRITE);
omap3isp_subclk_disable(isp, OMAP3_ISP_SUBCLK_PREVIEW);
- spin_unlock_irqrestore(&prev->lock, flags);
isp_video_dmaqueue_flags_clr(video_out);
break;
}
@@ -2201,7 +2300,7 @@
}
/*
- * isp_preview_init - Previewer initialization.
+ * omap3isp_preview_init - Previewer initialization.
* @dev : Pointer to ISP device
* return -ENOMEM or zero on success
*/
@@ -2209,8 +2308,8 @@
{
struct isp_prev_device *prev = &isp->isp_prev;
- spin_lock_init(&prev->lock);
init_waitqueue_head(&prev->wait);
+
preview_init_params(prev);
return preview_init_entities(prev);