| Message ID | 20200815103734.31153-9-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: staging: rkisp1: various bug fixes | expand |
Hi Dafna, On 8/15/20 7:37 AM, Dafna Hirschfeld wrote: > The params isr is called when a frame is out of the isp. The parameters > are applied immediately since the isr updates the shadow registers. > Therefore the params are first applied on the next frame. > We want the vb.sequence to be the frame that the params are applied to. > So we set vb.sequence to be the isp's frame_sequence + 1 > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > drivers/staging/media/rkisp1/rkisp1-params.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c > index 134b5c9a94c1..4b4391c0a2a0 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-params.c > +++ b/drivers/staging/media/rkisp1/rkisp1-params.c > @@ -1220,7 +1220,14 @@ void rkisp1_params_apply_params_cfg(struct rkisp1_params *params, unsigned int f > > void rkisp1_params_isr(struct rkisp1_device *rkisp1) > { > - unsigned int frame_sequence = atomic_read(&rkisp1->isp.frame_sequence); > + /* > + * The params isr is called when a frame is out of the isp. The parameters > + * are applied immediately since the isr updates the shadow registers. > + * Therefore the params are first applied on the next frame. > + * We want the vb.sequence to be the frame that the params are applied to. > + * So we set vb.sequence to be the isp's frame_sequence + 1 > + */ I would just re-phrase this a bit, how about: This isr is called when the ISP finishes processing a frame. To configure the parameters, we update the shadow registers, which means that the next frame will already take these new configuration into consideration. Since frame_sequence is only updated on the vertical sync signal, we should use frame_sequence + 1 here to indicate to userspace which frame this parameters are being applied to. Or maybe smaller: This isr is called when the ISP finishes processing a frame. Configurations performed here will be applied to the next frame. Since frame_sequence is only updated on the vertical sync signal, we should use frame_sequence + 1 here to indicate to userspace which frame this parameters are being applied to. What do you think? With an improvement in the text (and also commit message): Acked-by: Helen Koike <helen.koike@collabora.com> Regards, Helen > + unsigned int frame_sequence = atomic_read(&rkisp1->isp.frame_sequence) + 1; > struct rkisp1_params *params = &rkisp1->params; > > spin_lock(¶ms->config_lock); >
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c index 134b5c9a94c1..4b4391c0a2a0 100644 --- a/drivers/staging/media/rkisp1/rkisp1-params.c +++ b/drivers/staging/media/rkisp1/rkisp1-params.c @@ -1220,7 +1220,14 @@ void rkisp1_params_apply_params_cfg(struct rkisp1_params *params, unsigned int f void rkisp1_params_isr(struct rkisp1_device *rkisp1) { - unsigned int frame_sequence = atomic_read(&rkisp1->isp.frame_sequence); + /* + * The params isr is called when a frame is out of the isp. The parameters + * are applied immediately since the isr updates the shadow registers. + * Therefore the params are first applied on the next frame. + * We want the vb.sequence to be the frame that the params are applied to. + * So we set vb.sequence to be the isp's frame_sequence + 1 + */ + unsigned int frame_sequence = atomic_read(&rkisp1->isp.frame_sequence) + 1; struct rkisp1_params *params = &rkisp1->params; spin_lock(¶ms->config_lock);
The params isr is called when a frame is out of the isp. The parameters are applied immediately since the isr updates the shadow registers. Therefore the params are first applied on the next frame. We want the vb.sequence to be the frame that the params are applied to. So we set vb.sequence to be the isp's frame_sequence + 1 Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> --- drivers/staging/media/rkisp1/rkisp1-params.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)