| Message ID | 20200509152904.26348-2-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] media: staging: rkisp1 stats: set a measure flag with '|=' instead of '=' | expand |
On 5/9/20 12:29 PM, Dafna Hirschfeld wrote: > The flags that indicate which statistics are read are already > set in the functions that read them so there is no need to > set them in the function rkisp1_stats_send_measurement. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Acked-by: Helen Koike <helen.koike@collabora.com> Thanks Helen > --- > drivers/staging/media/rkisp1/rkisp1-stats.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c > index 8351bda0be03..0616793ae395 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-stats.c > +++ b/drivers/staging/media/rkisp1/rkisp1-stats.c > @@ -356,26 +356,19 @@ rkisp1_stats_send_measurement(struct rkisp1_stats *stats, > cur_stat_buf = > (struct rkisp1_stat_buffer *)(cur_buf->vaddr[0]); > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) > rkisp1_stats_get_awb_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AWB; > - } > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) > rkisp1_stats_get_afc_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AFM_FIN; > - } > > if (meas_work->isp_ris & RKISP1_CIF_ISP_EXP_END) { > rkisp1_stats_get_aec_meas(stats, cur_stat_buf); > rkisp1_stats_get_bls_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AUTOEXP; > } > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) > rkisp1_stats_get_hst_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_HIST; > - } > > vb2_set_plane_payload(&cur_buf->vb.vb2_buf, 0, > sizeof(struct rkisp1_stat_buffer)); >
Hi Dafna, Thank you for the patch. On Sat, May 09, 2020 at 05:29:04PM +0200, Dafna Hirschfeld wrote: > The flags that indicate which statistics are read are already > set in the functions that read them so there is no need to > set them in the function rkisp1_stats_send_measurement. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/staging/media/rkisp1/rkisp1-stats.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c > index 8351bda0be03..0616793ae395 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-stats.c > +++ b/drivers/staging/media/rkisp1/rkisp1-stats.c > @@ -356,26 +356,19 @@ rkisp1_stats_send_measurement(struct rkisp1_stats *stats, > cur_stat_buf = > (struct rkisp1_stat_buffer *)(cur_buf->vaddr[0]); > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) > rkisp1_stats_get_awb_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AWB; > - } > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) > rkisp1_stats_get_afc_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AFM_FIN; > - } > > if (meas_work->isp_ris & RKISP1_CIF_ISP_EXP_END) { > rkisp1_stats_get_aec_meas(stats, cur_stat_buf); > rkisp1_stats_get_bls_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AUTOEXP; > } > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) > rkisp1_stats_get_hst_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_HIST; > - } > > vb2_set_plane_payload(&cur_buf->vb.vb2_buf, 0, > sizeof(struct rkisp1_stat_buffer));
On Sat, May 9, 2020 at 5:29 PM Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > The flags that indicate which statistics are read are already > set in the functions that read them so there is no need to > set them in the function rkisp1_stats_send_measurement. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > drivers/staging/media/rkisp1/rkisp1-stats.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c > index 8351bda0be03..0616793ae395 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-stats.c > +++ b/drivers/staging/media/rkisp1/rkisp1-stats.c > @@ -356,26 +356,19 @@ rkisp1_stats_send_measurement(struct rkisp1_stats *stats, > cur_stat_buf = > (struct rkisp1_stat_buffer *)(cur_buf->vaddr[0]); > Should we perhaps also set cur_stat_buf->meas_type to 0 here? > - if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) > rkisp1_stats_get_awb_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AWB; I wonder if it wouldn't be cleaner if this function took care of meas_type, rather than sprinkling it all over the measurement collection functions. > - } > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) > rkisp1_stats_get_afc_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AFM_FIN; > - } > > if (meas_work->isp_ris & RKISP1_CIF_ISP_EXP_END) { > rkisp1_stats_get_aec_meas(stats, cur_stat_buf); > rkisp1_stats_get_bls_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AUTOEXP; > } > > - if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) { > + if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) > rkisp1_stats_get_hst_meas(stats, cur_stat_buf); > - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_HIST; > - } > > vb2_set_plane_payload(&cur_buf->vb.vb2_buf, 0, > sizeof(struct rkisp1_stat_buffer)); > -- > 2.17.1 >
diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c index 8351bda0be03..0616793ae395 100644 --- a/drivers/staging/media/rkisp1/rkisp1-stats.c +++ b/drivers/staging/media/rkisp1/rkisp1-stats.c @@ -356,26 +356,19 @@ rkisp1_stats_send_measurement(struct rkisp1_stats *stats, cur_stat_buf = (struct rkisp1_stat_buffer *)(cur_buf->vaddr[0]); - if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) { + if (meas_work->isp_ris & RKISP1_CIF_ISP_AWB_DONE) rkisp1_stats_get_awb_meas(stats, cur_stat_buf); - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AWB; - } - if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) { + if (meas_work->isp_ris & RKISP1_CIF_ISP_AFM_FIN) rkisp1_stats_get_afc_meas(stats, cur_stat_buf); - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AFM_FIN; - } if (meas_work->isp_ris & RKISP1_CIF_ISP_EXP_END) { rkisp1_stats_get_aec_meas(stats, cur_stat_buf); rkisp1_stats_get_bls_meas(stats, cur_stat_buf); - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_AUTOEXP; } - if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) { + if (meas_work->isp_ris & RKISP1_CIF_ISP_HIST_MEASURE_RDY) rkisp1_stats_get_hst_meas(stats, cur_stat_buf); - cur_stat_buf->meas_type |= RKISP1_CIF_ISP_STAT_HIST; - } vb2_set_plane_payload(&cur_buf->vb.vb2_buf, 0, sizeof(struct rkisp1_stat_buffer));
The flags that indicate which statistics are read are already set in the functions that read them so there is no need to set them in the function rkisp1_stats_send_measurement. Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> --- drivers/staging/media/rkisp1/rkisp1-stats.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)