| Message ID | 20200604190015.12069-2-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] media: staging: rkisp1: rsz: supported formats are the isp's src formats, not sink formats | expand |
On 6/4/20 4:00 PM, Dafna Hirschfeld wrote: > When setting the sink format of the 'rkisp1_resizer' > the format should be supported by 'rkisp1_isp' on > the video source pad. This patch checks this condition > and set the format to default if the condition is false. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > drivers/staging/media/rkisp1/rkisp1-common.h | 4 ++++ > drivers/staging/media/rkisp1/rkisp1-isp.c | 4 ---- > drivers/staging/media/rkisp1/rkisp1-resizer.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h > index 0c4fe503adc9..39d8e46d8d8a 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-common.h > +++ b/drivers/staging/media/rkisp1/rkisp1-common.h > @@ -22,6 +22,10 @@ > #include "rkisp1-regs.h" > #include "uapi/rkisp1-config.h" > > +#define RKISP1_DIR_SRC BIT(0) > +#define RKISP1_DIR_SINK BIT(1) > +#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC) I think we can rename this macro to state explicitly it is the direction from the rkisp1_isp subdevice node point of view. Maybe prefix it with RKISP1_ISP_SD_ ? Thanks Helen > + > #define RKISP1_ISP_MAX_WIDTH 4032 > #define RKISP1_ISP_MAX_HEIGHT 3024 > #define RKISP1_ISP_MIN_WIDTH 32 > diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c > index dc2b59a0160a..e66e87d6ea8b 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-isp.c > +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c > @@ -23,10 +23,6 @@ > > #define RKISP1_ISP_DEV_NAME RKISP1_DRIVER_NAME "_isp" > > -#define RKISP1_DIR_SRC BIT(0) > -#define RKISP1_DIR_SINK BIT(1) > -#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC) > - > /* > * NOTE: MIPI controller and input MUX are also configured in this file. > * This is because ISP Subdev describes not only ISP submodule (input size, > diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c > index d64c064bdb1d..fa28f4bd65c0 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c > +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c > @@ -542,7 +542,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz, > which); > sink_fmt->code = format->code; > mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code); > - if (!mbus_info) { > + if (!mbus_info || !(mbus_info->direction & RKISP1_DIR_SRC)) { > sink_fmt->code = RKISP1_DEF_FMT; > mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code); > } >
On 04.06.20 21:07, Helen Koike wrote: > > > On 6/4/20 4:00 PM, Dafna Hirschfeld wrote: >> When setting the sink format of the 'rkisp1_resizer' >> the format should be supported by 'rkisp1_isp' on >> the video source pad. This patch checks this condition >> and set the format to default if the condition is false. >> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >> --- >> drivers/staging/media/rkisp1/rkisp1-common.h | 4 ++++ >> drivers/staging/media/rkisp1/rkisp1-isp.c | 4 ---- >> drivers/staging/media/rkisp1/rkisp1-resizer.c | 2 +- >> 3 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h >> index 0c4fe503adc9..39d8e46d8d8a 100644 >> --- a/drivers/staging/media/rkisp1/rkisp1-common.h >> +++ b/drivers/staging/media/rkisp1/rkisp1-common.h >> @@ -22,6 +22,10 @@ >> #include "rkisp1-regs.h" >> #include "uapi/rkisp1-config.h" >> >> +#define RKISP1_DIR_SRC BIT(0) >> +#define RKISP1_DIR_SINK BIT(1) >> +#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC) > > I think we can rename this macro to state explicitly it is the direction from the rkisp1_isp subdevice node > point of view. > > Maybe prefix it with RKISP1_ISP_SD_ ? yes, it's a better name. > > Thanks > Helen > >> + >> #define RKISP1_ISP_MAX_WIDTH 4032 >> #define RKISP1_ISP_MAX_HEIGHT 3024 >> #define RKISP1_ISP_MIN_WIDTH 32 >> diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c >> index dc2b59a0160a..e66e87d6ea8b 100644 >> --- a/drivers/staging/media/rkisp1/rkisp1-isp.c >> +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c >> @@ -23,10 +23,6 @@ >> >> #define RKISP1_ISP_DEV_NAME RKISP1_DRIVER_NAME "_isp" >> >> -#define RKISP1_DIR_SRC BIT(0) >> -#define RKISP1_DIR_SINK BIT(1) >> -#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC) >> - >> /* >> * NOTE: MIPI controller and input MUX are also configured in this file. >> * This is because ISP Subdev describes not only ISP submodule (input size, >> diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c >> index d64c064bdb1d..fa28f4bd65c0 100644 >> --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c >> +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c >> @@ -542,7 +542,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz, >> which); >> sink_fmt->code = format->code; >> mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code); >> - if (!mbus_info) { >> + if (!mbus_info || !(mbus_info->direction & RKISP1_DIR_SRC)) { I also think the field 'direction' better change to isp_pads_flags. It indicates better what this field is. Thanks, Dafna >> sink_fmt->code = RKISP1_DEF_FMT; >> mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code); >> } >>
diff --git a/drivers/staging/media/rkisp1/rkisp1-common.h b/drivers/staging/media/rkisp1/rkisp1-common.h index 0c4fe503adc9..39d8e46d8d8a 100644 --- a/drivers/staging/media/rkisp1/rkisp1-common.h +++ b/drivers/staging/media/rkisp1/rkisp1-common.h @@ -22,6 +22,10 @@ #include "rkisp1-regs.h" #include "uapi/rkisp1-config.h" +#define RKISP1_DIR_SRC BIT(0) +#define RKISP1_DIR_SINK BIT(1) +#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC) + #define RKISP1_ISP_MAX_WIDTH 4032 #define RKISP1_ISP_MAX_HEIGHT 3024 #define RKISP1_ISP_MIN_WIDTH 32 diff --git a/drivers/staging/media/rkisp1/rkisp1-isp.c b/drivers/staging/media/rkisp1/rkisp1-isp.c index dc2b59a0160a..e66e87d6ea8b 100644 --- a/drivers/staging/media/rkisp1/rkisp1-isp.c +++ b/drivers/staging/media/rkisp1/rkisp1-isp.c @@ -23,10 +23,6 @@ #define RKISP1_ISP_DEV_NAME RKISP1_DRIVER_NAME "_isp" -#define RKISP1_DIR_SRC BIT(0) -#define RKISP1_DIR_SINK BIT(1) -#define RKISP1_DIR_SINK_SRC (RKISP1_DIR_SINK | RKISP1_DIR_SRC) - /* * NOTE: MIPI controller and input MUX are also configured in this file. * This is because ISP Subdev describes not only ISP submodule (input size, diff --git a/drivers/staging/media/rkisp1/rkisp1-resizer.c b/drivers/staging/media/rkisp1/rkisp1-resizer.c index d64c064bdb1d..fa28f4bd65c0 100644 --- a/drivers/staging/media/rkisp1/rkisp1-resizer.c +++ b/drivers/staging/media/rkisp1/rkisp1-resizer.c @@ -542,7 +542,7 @@ static void rkisp1_rsz_set_sink_fmt(struct rkisp1_resizer *rsz, which); sink_fmt->code = format->code; mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code); - if (!mbus_info) { + if (!mbus_info || !(mbus_info->direction & RKISP1_DIR_SRC)) { sink_fmt->code = RKISP1_DEF_FMT; mbus_info = rkisp1_isp_mbus_info_get(sink_fmt->code); }
When setting the sink format of the 'rkisp1_resizer' the format should be supported by 'rkisp1_isp' on the video source pad. This patch checks this condition and set the format to default if the condition is false. Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> --- drivers/staging/media/rkisp1/rkisp1-common.h | 4 ++++ drivers/staging/media/rkisp1/rkisp1-isp.c | 4 ---- drivers/staging/media/rkisp1/rkisp1-resizer.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)