| Message ID | 8dbc93c2a7291d942d2d37491833444d77316211.1567822793.git.skhan@linuxfoundation.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Collapse vimc into single monolithic driver | expand |
Hi Shuah, On 9/6/19 11:42 PM, Shuah Khan wrote: > Move duplicated IS_SRC and IS_SINK dfines to common header. Rename > them to VIMC_IS_SRC and VIM_IS_SINK. > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> > --- > drivers/media/platform/vimc/vimc-common.h | 4 ++++ > drivers/media/platform/vimc/vimc-debayer.c | 11 ++++------- > drivers/media/platform/vimc/vimc-scaler.c | 8 +++----- > 3 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h > index 87ee84f78322..236412ad7548 100644 > --- a/drivers/media/platform/vimc/vimc-common.h > +++ b/drivers/media/platform/vimc/vimc-common.h > @@ -27,6 +27,10 @@ > > #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp) > > +/* Source and sink pad checks */ > +#define VIMC_IS_SRC(pad) (pad) > +#define VIMC_IS_SINK(pad) (!(pad)) This is true now, but it might not be true in the future. In the output video device (that was sent by André but not yet upstream) for instance, only have a single source pad (which I suppose the index will be 0), and this macro won't be true. Maybe we could check pad flags in sd->entity->pads[index].flags ? Thanks Helen > + > /** > * struct vimc_colorimetry_clamp - Adjust colorimetry parameters > * > diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c > index b38b55f51a24..37f3767db469 100644 > --- a/drivers/media/platform/vimc/vimc-debayer.c > +++ b/drivers/media/platform/vimc/vimc-debayer.c > @@ -22,9 +22,6 @@ MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n" > "stays in the center of the window, otherwise the next odd number " > "is considered"); > > -#define IS_SINK(pad) (!pad) > -#define IS_SRC(pad) (pad) > - > enum vimc_deb_rgb_colors { > VIMC_DEB_RED = 0, > VIMC_DEB_GREEN = 1, > @@ -157,7 +154,7 @@ static int vimc_deb_enum_mbus_code(struct v4l2_subdev *sd, > struct v4l2_subdev_mbus_code_enum *code) > { > /* We only support one format for source pads */ > - if (IS_SRC(code->pad)) { > + if (VIMC_IS_SRC(code->pad)) { > struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd); > > if (code->index) > @@ -183,7 +180,7 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd, > if (fse->index) > return -EINVAL; > > - if (IS_SINK(fse->pad)) { > + if (VIMC_IS_SINK(fse->pad)) { > const struct vimc_deb_pix_map *vpix = > vimc_deb_pix_map_by_code(fse->code); > > @@ -213,7 +210,7 @@ static int vimc_deb_get_fmt(struct v4l2_subdev *sd, > vdeb->sink_fmt; > > /* Set the right code for the source pad */ > - if (IS_SRC(fmt->pad)) > + if (VIMC_IS_SRC(fmt->pad)) > fmt->format.code = vdeb->src_code; > > return 0; > @@ -260,7 +257,7 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd, > * Do not change the format of the source pad, > * it is propagated from the sink > */ > - if (IS_SRC(fmt->pad)) { > + if (VIMC_IS_SRC(fmt->pad)) { > fmt->format = *sink_fmt; > /* TODO: Add support for other formats */ > fmt->format.code = vdeb->src_code; > diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c > index 05db5070e268..a5a0855ad9cd 100644 > --- a/drivers/media/platform/vimc/vimc-scaler.c > +++ b/drivers/media/platform/vimc/vimc-scaler.c > @@ -16,8 +16,6 @@ static unsigned int sca_mult = 3; > module_param(sca_mult, uint, 0000); > MODULE_PARM_DESC(sca_mult, " the image size multiplier"); > > -#define IS_SINK(pad) (!pad) > -#define IS_SRC(pad) (pad) > #define MAX_ZOOM 8 > > struct vimc_sca_device { > @@ -93,7 +91,7 @@ static int vimc_sca_enum_frame_size(struct v4l2_subdev *sd, > fse->min_width = VIMC_FRAME_MIN_WIDTH; > fse->min_height = VIMC_FRAME_MIN_HEIGHT; > > - if (IS_SINK(fse->pad)) { > + if (VIMC_IS_SINK(fse->pad)) { > fse->max_width = VIMC_FRAME_MAX_WIDTH; > fse->max_height = VIMC_FRAME_MAX_HEIGHT; > } else { > @@ -116,7 +114,7 @@ static int vimc_sca_get_fmt(struct v4l2_subdev *sd, > vsca->sink_fmt; > > /* Scale the frame size for the source pad */ > - if (IS_SRC(format->pad)) { > + if (VIMC_IS_SRC(format->pad)) { > format->format.width = vsca->sink_fmt.width * sca_mult; > format->format.height = vsca->sink_fmt.height * sca_mult; > } > @@ -165,7 +163,7 @@ static int vimc_sca_set_fmt(struct v4l2_subdev *sd, > * Do not change the format of the source pad, > * it is propagated from the sink > */ > - if (IS_SRC(fmt->pad)) { > + if (VIMC_IS_SRC(fmt->pad)) { > fmt->format = *sink_fmt; > fmt->format.width = sink_fmt->width * sca_mult; > fmt->format.height = sink_fmt->height * sca_mult; >
On 9/15/19 1:25 PM, Helen Koike wrote: > Hi Shuah, > > On 9/6/19 11:42 PM, Shuah Khan wrote: >> Move duplicated IS_SRC and IS_SINK dfines to common header. Rename >> them to VIMC_IS_SRC and VIM_IS_SINK. >> >> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> >> --- >> drivers/media/platform/vimc/vimc-common.h | 4 ++++ >> drivers/media/platform/vimc/vimc-debayer.c | 11 ++++------- >> drivers/media/platform/vimc/vimc-scaler.c | 8 +++----- >> 3 files changed, 11 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h >> index 87ee84f78322..236412ad7548 100644 >> --- a/drivers/media/platform/vimc/vimc-common.h >> +++ b/drivers/media/platform/vimc/vimc-common.h >> @@ -27,6 +27,10 @@ >> >> #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp) >> >> +/* Source and sink pad checks */ >> +#define VIMC_IS_SRC(pad) (pad) >> +#define VIMC_IS_SINK(pad) (!(pad)) > > This is true now, but it might not be true in the future. > In the output video device (that was sent by André but not yet upstream) for instance, only have a single > source pad (which I suppose the index will be 0), and this macro won't be true. > > Maybe we could check pad flags in sd->entity->pads[index].flags ? > I think this change should be done in André's patch? thanks, -- Shuah
On 9/15/19 8:52 PM, Shuah Khan wrote: > On 9/15/19 1:25 PM, Helen Koike wrote: >> Hi Shuah, >> >> On 9/6/19 11:42 PM, Shuah Khan wrote: >>> Move duplicated IS_SRC and IS_SINK dfines to common header. Rename >>> them to VIMC_IS_SRC and VIM_IS_SINK. >>> >>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> >>> --- >>> drivers/media/platform/vimc/vimc-common.h | 4 ++++ >>> drivers/media/platform/vimc/vimc-debayer.c | 11 ++++------- >>> drivers/media/platform/vimc/vimc-scaler.c | 8 +++----- >>> 3 files changed, 11 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h >>> index 87ee84f78322..236412ad7548 100644 >>> --- a/drivers/media/platform/vimc/vimc-common.h >>> +++ b/drivers/media/platform/vimc/vimc-common.h >>> @@ -27,6 +27,10 @@ >>> #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp) >>> +/* Source and sink pad checks */ >>> +#define VIMC_IS_SRC(pad) (pad) >>> +#define VIMC_IS_SINK(pad) (!(pad)) >> >> This is true now, but it might not be true in the future. >> In the output video device (that was sent by André but not yet upstream) for instance, only have a single >> source pad (which I suppose the index will be 0), and this macro won't be true. >> > >> Maybe we could check pad flags in sd->entity->pads[index].flags ? >> > > I think this change should be done in André's patch? Could be yes, making it generic since the start would be nice, but I don't mind updating this latter when needed. Then: Acked-by: Helen Koike <helen.koike@collabora.com> > > thanks, > -- Shuah Thanks Helen
On 9/16/19 4:42 AM, Helen Koike wrote: > > > On 9/15/19 8:52 PM, Shuah Khan wrote: >> On 9/15/19 1:25 PM, Helen Koike wrote: >>> Hi Shuah, >>> >>> On 9/6/19 11:42 PM, Shuah Khan wrote: >>>> Move duplicated IS_SRC and IS_SINK dfines to common header. Rename >>>> them to VIMC_IS_SRC and VIM_IS_SINK. >>>> >>>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> >>>> --- >>>> drivers/media/platform/vimc/vimc-common.h | 4 ++++ >>>> drivers/media/platform/vimc/vimc-debayer.c | 11 ++++------- >>>> drivers/media/platform/vimc/vimc-scaler.c | 8 +++----- >>>> 3 files changed, 11 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h >>>> index 87ee84f78322..236412ad7548 100644 >>>> --- a/drivers/media/platform/vimc/vimc-common.h >>>> +++ b/drivers/media/platform/vimc/vimc-common.h >>>> @@ -27,6 +27,10 @@ >>>> #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp) >>>> +/* Source and sink pad checks */ >>>> +#define VIMC_IS_SRC(pad) (pad) >>>> +#define VIMC_IS_SINK(pad) (!(pad)) >>> >>> This is true now, but it might not be true in the future. >>> In the output video device (that was sent by André but not yet upstream) for instance, only have a single >>> source pad (which I suppose the index will be 0), and this macro won't be true. >>> >> >>> Maybe we could check pad flags in sd->entity->pads[index].flags ? >>> >> >> I think this change should be done in André's patch? > > Could be yes, making it generic since the start would be nice, > but I don't mind updating this latter when needed. Let's go with that then. This way we can get this series in and then we can clean this up in André's patch. > > Then: > > Acked-by: Helen Koike <helen.koike@collabora.com> > >> thanks, -- Shuah
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h index 87ee84f78322..236412ad7548 100644 --- a/drivers/media/platform/vimc/vimc-common.h +++ b/drivers/media/platform/vimc/vimc-common.h @@ -27,6 +27,10 @@ #define VIMC_FRAME_INDEX(lin, col, width, bpp) ((lin * width + col) * bpp) +/* Source and sink pad checks */ +#define VIMC_IS_SRC(pad) (pad) +#define VIMC_IS_SINK(pad) (!(pad)) + /** * struct vimc_colorimetry_clamp - Adjust colorimetry parameters * diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c index b38b55f51a24..37f3767db469 100644 --- a/drivers/media/platform/vimc/vimc-debayer.c +++ b/drivers/media/platform/vimc/vimc-debayer.c @@ -22,9 +22,6 @@ MODULE_PARM_DESC(deb_mean_win_size, " the window size to calculate the mean.\n" "stays in the center of the window, otherwise the next odd number " "is considered"); -#define IS_SINK(pad) (!pad) -#define IS_SRC(pad) (pad) - enum vimc_deb_rgb_colors { VIMC_DEB_RED = 0, VIMC_DEB_GREEN = 1, @@ -157,7 +154,7 @@ static int vimc_deb_enum_mbus_code(struct v4l2_subdev *sd, struct v4l2_subdev_mbus_code_enum *code) { /* We only support one format for source pads */ - if (IS_SRC(code->pad)) { + if (VIMC_IS_SRC(code->pad)) { struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd); if (code->index) @@ -183,7 +180,7 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd, if (fse->index) return -EINVAL; - if (IS_SINK(fse->pad)) { + if (VIMC_IS_SINK(fse->pad)) { const struct vimc_deb_pix_map *vpix = vimc_deb_pix_map_by_code(fse->code); @@ -213,7 +210,7 @@ static int vimc_deb_get_fmt(struct v4l2_subdev *sd, vdeb->sink_fmt; /* Set the right code for the source pad */ - if (IS_SRC(fmt->pad)) + if (VIMC_IS_SRC(fmt->pad)) fmt->format.code = vdeb->src_code; return 0; @@ -260,7 +257,7 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd, * Do not change the format of the source pad, * it is propagated from the sink */ - if (IS_SRC(fmt->pad)) { + if (VIMC_IS_SRC(fmt->pad)) { fmt->format = *sink_fmt; /* TODO: Add support for other formats */ fmt->format.code = vdeb->src_code; diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c index 05db5070e268..a5a0855ad9cd 100644 --- a/drivers/media/platform/vimc/vimc-scaler.c +++ b/drivers/media/platform/vimc/vimc-scaler.c @@ -16,8 +16,6 @@ static unsigned int sca_mult = 3; module_param(sca_mult, uint, 0000); MODULE_PARM_DESC(sca_mult, " the image size multiplier"); -#define IS_SINK(pad) (!pad) -#define IS_SRC(pad) (pad) #define MAX_ZOOM 8 struct vimc_sca_device { @@ -93,7 +91,7 @@ static int vimc_sca_enum_frame_size(struct v4l2_subdev *sd, fse->min_width = VIMC_FRAME_MIN_WIDTH; fse->min_height = VIMC_FRAME_MIN_HEIGHT; - if (IS_SINK(fse->pad)) { + if (VIMC_IS_SINK(fse->pad)) { fse->max_width = VIMC_FRAME_MAX_WIDTH; fse->max_height = VIMC_FRAME_MAX_HEIGHT; } else { @@ -116,7 +114,7 @@ static int vimc_sca_get_fmt(struct v4l2_subdev *sd, vsca->sink_fmt; /* Scale the frame size for the source pad */ - if (IS_SRC(format->pad)) { + if (VIMC_IS_SRC(format->pad)) { format->format.width = vsca->sink_fmt.width * sca_mult; format->format.height = vsca->sink_fmt.height * sca_mult; } @@ -165,7 +163,7 @@ static int vimc_sca_set_fmt(struct v4l2_subdev *sd, * Do not change the format of the source pad, * it is propagated from the sink */ - if (IS_SRC(fmt->pad)) { + if (VIMC_IS_SRC(fmt->pad)) { fmt->format = *sink_fmt; fmt->format.width = sink_fmt->width * sca_mult; fmt->format.height = sink_fmt->height * sca_mult;
Move duplicated IS_SRC and IS_SINK dfines to common header. Rename them to VIMC_IS_SRC and VIM_IS_SINK. Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> --- drivers/media/platform/vimc/vimc-common.h | 4 ++++ drivers/media/platform/vimc/vimc-debayer.c | 11 ++++------- drivers/media/platform/vimc/vimc-scaler.c | 8 +++----- 3 files changed, 11 insertions(+), 12 deletions(-)