* [PATCH] media: staging/intel-ipu3: css: simplify expression
@ 2020-03-13 18:13 Deepak R Varma
2020-03-13 18:21 ` [Outreachy kernel] " Helen Koike
0 siblings, 1 reply; 4+ messages in thread
From: Deepak R Varma @ 2020-03-13 18:13 UTC (permalink / raw)
To: outreachy-kernel; +Cc: gregkh, daniel.baluta, mchehab, sakari.ailus
An array index computed inside square brackets leading to complexity
and code line exceeding 80 character. Add new variable to compute
array index separately and use it as an index during assignment.
Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
drivers/staging/media/ipu3/ipu3-css-params.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
index 4533dacad4be..4b15e767bf32 100644
--- a/drivers/staging/media/ipu3/ipu3-css-params.c
+++ b/drivers/staging/media/ipu3/ipu3-css-params.c
@@ -50,12 +50,13 @@ imgu_css_scaler_setup_lut(unsigned int taps, unsigned int input_width,
int exponent = imgu_css_scaler_get_exp(output_width, input_width);
int mantissa = (1 << exponent) * output_width;
unsigned int phase_step;
+ int phase_taps = 0;
if (input_width == output_width) {
for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
+ phase_taps = phase * IMGU_SCALER_FILTER_TAPS;
for (tap = 0; tap < taps; tap++) {
- coeff_lut[phase * IMGU_SCALER_FILTER_TAPS + tap]
- = 0;
+ coeff_lut[phase_taps + tap] = 0;
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Outreachy kernel] [PATCH] media: staging/intel-ipu3: css: simplify expression
2020-03-13 18:13 [PATCH] media: staging/intel-ipu3: css: simplify expression Deepak R Varma
@ 2020-03-13 18:21 ` Helen Koike
2020-03-17 1:05 ` Stefano Brivio
0 siblings, 1 reply; 4+ messages in thread
From: Helen Koike @ 2020-03-13 18:21 UTC (permalink / raw)
To: Deepak R Varma, outreachy-kernel
Cc: gregkh, daniel.baluta, mchehab, sakari.ailus
Hi Deepak,
On 3/13/20 3:13 PM, Deepak R Varma wrote:
> An array index computed inside square brackets leading to complexity
> and code line exceeding 80 character. Add new variable to compute
> array index separately and use it as an index during assignment.
>
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
> drivers/staging/media/ipu3/ipu3-css-params.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> index 4533dacad4be..4b15e767bf32 100644
> --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> @@ -50,12 +50,13 @@ imgu_css_scaler_setup_lut(unsigned int taps, unsigned int input_width,
> int exponent = imgu_css_scaler_get_exp(output_width, input_width);
> int mantissa = (1 << exponent) * output_width;
> unsigned int phase_step;
> + int phase_taps = 0;
No need to initialized it to zero.
Also, it seems phase_taps can't be negative, so you can use unsigned int
>
> if (input_width == output_width) {
> for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
> + phase_taps = phase * IMGU_SCALER_FILTER_TAPS;
You can also declare it here:
+ unsigned int offset = phase * IMGU_SCALER_FILTER_TAPS;
> for (tap = 0; tap < taps; tap++) {
> - coeff_lut[phase * IMGU_SCALER_FILTER_TAPS + tap]
> - = 0;
> + coeff_lut[phase_taps + tap] = 0;
> }
> }
>
>
Regards
Helen
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Outreachy kernel] [PATCH] media: staging/intel-ipu3: css: simplify expression
2020-03-13 18:21 ` [Outreachy kernel] " Helen Koike
@ 2020-03-17 1:05 ` Stefano Brivio
2020-03-17 1:22 ` Stefano Brivio
0 siblings, 1 reply; 4+ messages in thread
From: Stefano Brivio @ 2020-03-17 1:05 UTC (permalink / raw)
To: Deepak R Varma
Cc: Helen Koike, outreachy-kernel, gregkh, daniel.baluta, mchehab,
sakari.ailus
On Fri, 13 Mar 2020 15:21:28 -0300
Helen Koike <helen@koikeco.de> wrote:
> Hi Deepak,
>
> On 3/13/20 3:13 PM, Deepak R Varma wrote:
> > An array index computed inside square brackets leading to complexity
> > and code line exceeding 80 character. Add new variable to compute
The first statement isn't really a statement ("leads" instead of
"leading"?)
> > array index separately and use it as an index during assignment.
> >
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > ---
> > drivers/staging/media/ipu3/ipu3-css-params.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> > index 4533dacad4be..4b15e767bf32 100644
> > --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> > @@ -50,12 +50,13 @@ imgu_css_scaler_setup_lut(unsigned int taps, unsigned int input_width,
> > int exponent = imgu_css_scaler_get_exp(output_width, input_width);
> > int mantissa = (1 << exponent) * output_width;
> > unsigned int phase_step;
> > + int phase_taps = 0;
>
> No need to initialized it to zero.
>
> Also, it seems phase_taps can't be negative, so you can use unsigned int
>
> >
> > if (input_width == output_width) {
> > for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
> > + phase_taps = phase * IMGU_SCALER_FILTER_TAPS;
>
> You can also declare it here:
> + unsigned int offset = phase * IMGU_SCALER_FILTER_TAPS;
>
> > for (tap = 0; tap < taps; tap++) {
Also, at that point, you should extend this to the loop for
input_width != output_width below, for consistency.
> > - coeff_lut[phase * IMGU_SCALER_FILTER_TAPS + tap]
> > - = 0;
> > + coeff_lut[phase_taps + tap] = 0;
--
Stefano
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Outreachy kernel] [PATCH] media: staging/intel-ipu3: css: simplify expression
2020-03-17 1:05 ` Stefano Brivio
@ 2020-03-17 1:22 ` Stefano Brivio
0 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2020-03-17 1:22 UTC (permalink / raw)
To: Deepak R Varma
Cc: Helen Koike, outreachy-kernel, gregkh, daniel.baluta, mchehab,
sakari.ailus
On Tue, 17 Mar 2020 02:05:58 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:
> On Fri, 13 Mar 2020 15:21:28 -0300
> Helen Koike <helen@koikeco.de> wrote:
>
> > Hi Deepak,
> >
> > On 3/13/20 3:13 PM, Deepak R Varma wrote:
> > > An array index computed inside square brackets leading to complexity
> > > and code line exceeding 80 character. Add new variable to compute
>
> The first statement isn't really a statement ("leads" instead of
> "leading"?)
>
> > > array index separately and use it as an index during assignment.
> > >
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > ---
> > > drivers/staging/media/ipu3/ipu3-css-params.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/ipu3/ipu3-css-params.c b/drivers/staging/media/ipu3/ipu3-css-params.c
> > > index 4533dacad4be..4b15e767bf32 100644
> > > --- a/drivers/staging/media/ipu3/ipu3-css-params.c
> > > +++ b/drivers/staging/media/ipu3/ipu3-css-params.c
> > > @@ -50,12 +50,13 @@ imgu_css_scaler_setup_lut(unsigned int taps, unsigned int input_width,
> > > int exponent = imgu_css_scaler_get_exp(output_width, input_width);
> > > int mantissa = (1 << exponent) * output_width;
> > > unsigned int phase_step;
> > > + int phase_taps = 0;
> >
> > No need to initialized it to zero.
> >
> > Also, it seems phase_taps can't be negative, so you can use unsigned int
> >
> > >
> > > if (input_width == output_width) {
> > > for (phase = 0; phase < IMGU_SCALER_PHASES; phase++) {
> > > + phase_taps = phase * IMGU_SCALER_FILTER_TAPS;
> >
> > You can also declare it here:
> > + unsigned int offset = phase * IMGU_SCALER_FILTER_TAPS;
> >
> > > for (tap = 0; tap < taps; tap++) {
>
> Also, at that point, you should extend this to the loop for
> input_width != output_width below, for consistency.
Sorry, I missed v2 and v3 of this, never mind.
--
Stefano
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-17 1:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-13 18:13 [PATCH] media: staging/intel-ipu3: css: simplify expression Deepak R Varma
2020-03-13 18:21 ` [Outreachy kernel] " Helen Koike
2020-03-17 1:05 ` Stefano Brivio
2020-03-17 1:22 ` Stefano Brivio
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.