| Message ID | 20210315155942.640889-1-robert.foss@linaro.org |
|---|---|
| Headers | show |
| Series | Add support for the SDM845 Camera Subsystem | expand |
On 15/03/2021 16:59, Robert Foss wrote: > In order to support Qualcomm ISP hardware architectures that diverge > from older architectures, the CSID subdevice drivers needs to be refactored > to better abstract the different ISP hardware architectures. > > Signed-off-by: Robert Foss <robert.foss@linaro.org> > Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org> > --- > > > Changes since v1: > - kernel test robot: Add missing include, interrupt.h > > Changes since v4: > - Andrey: Removed whitespace from some includes > - Andrey: Removed unused enum > > Changes since v5: > - Andrey: Fixed test pattern selection logic > - Andrey: Align test mode enum values with v4l mode selection return values > - Andrey: r-b > - Move Titan 170 test modes to the the Titan 170 commit > - Fixed test pattern boundary check > > Changes since v7: > - Hans: Fix checkpatch.pl --strict warnings > > > > drivers/media/platform/qcom/camss/Makefile | 2 + > .../platform/qcom/camss/camss-csid-4-1.c | 328 ++++++++++ > .../platform/qcom/camss/camss-csid-4-7.c | 404 ++++++++++++ > .../media/platform/qcom/camss/camss-csid.c | 608 +----------------- > .../media/platform/qcom/camss/camss-csid.h | 129 +++- > 5 files changed, 885 insertions(+), 586 deletions(-) > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-1.c > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-7.c > <snip> > diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h > index 479ac1f83836..613ef377b051 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.h > +++ b/drivers/media/platform/qcom/camss/camss-csid.h > @@ -11,6 +11,7 @@ > #define QC_MSM_CAMSS_CSID_H > > #include <linux/clk.h> > +#include <linux/interrupt.h> > #include <media/media-entity.h> > #include <media/v4l2-ctrls.h> > #include <media/v4l2-device.h> > @@ -44,18 +45,42 @@ > #define DATA_TYPE_RAW_16BIT 0x2e > #define DATA_TYPE_RAW_20BIT 0x2f > > -enum csid_payload_mode { > - CSID_PAYLOAD_MODE_INCREMENTING = 0, > - CSID_PAYLOAD_MODE_ALTERNATING_55_AA = 1, > - CSID_PAYLOAD_MODE_ALL_ZEROES = 2, > - CSID_PAYLOAD_MODE_ALL_ONES = 3, > - CSID_PAYLOAD_MODE_RANDOM = 4, > - CSID_PAYLOAD_MODE_USER_SPECIFIED = 5, > +#define CSID_RESET_TIMEOUT_MS 500 > + > +enum csid_testgen_mode { > + CSID_PAYLOAD_MODE_DISABLED = 0, > + CSID_PAYLOAD_MODE_INCREMENTING = 1, > + CSID_PAYLOAD_MODE_ALTERNATING_55_AA = 2, > + CSID_PAYLOAD_MODE_ALL_ZEROES = 3, > + CSID_PAYLOAD_MODE_ALL_ONES = 4, > + CSID_PAYLOAD_MODE_RANDOM = 5, > + CSID_PAYLOAD_MODE_USER_SPECIFIED = 6, > + CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 6, /* excluding disabled */ > +}; > + > +static const char * const csid_testgen_modes[] = { > + "Disabled", > + "Incrementing", > + "Alternating 0x55/0xAA", > + "All Zeros 0x00", > + "All Ones 0xFF", > + "Pseudo-random Data", > + "User Specified", > +}; This gives this sparse warning: 'csid_testgen_modes' defined but not used [-Wunused-const-variable=] This array needs to be moved to camss-csid.c and declared as an extern here. Also, this menu array needs to be terminated with a NULL, and the right capitalization needs to be used (first character of each word must be a capital). This is a suggested patch I made to verify that this solves this issue, but really both patch 9 and 10 need to be modified. Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> --- drivers/media/platform/qcom/camss/camss-csid.c | 14 ++++++++++++++ drivers/media/platform/qcom/camss/camss-csid.h | 13 +------------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c index fb94dc03ccd4..1513b3d47fc2 100644 --- a/drivers/media/platform/qcom/camss/camss-csid.c +++ b/drivers/media/platform/qcom/camss/camss-csid.c @@ -27,6 +27,20 @@ #define MSM_CSID_NAME "msm_csid" +const char * const csid_testgen_modes[] = { + "Disabled", + "Incrementing", + "Alternating 0x55/0xAA", + "All Zeros 0x00", + "All Ones 0xFF", + "Pseudo-Random Data", + "User Specified", + "Complex Pattern", + "Color Box", + "Color Bars", + NULL +}; + u32 csid_find_code(u32 *codes, unsigned int ncodes, unsigned int match_format_idx, u32 match_code) { diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h index c2a025f6846b..81a3704ac0e3 100644 --- a/drivers/media/platform/qcom/camss/camss-csid.h +++ b/drivers/media/platform/qcom/camss/camss-csid.h @@ -62,18 +62,7 @@ enum csid_testgen_mode { CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2 = 9, /* excluding disabled */ }; -static const char * const csid_testgen_modes[] = { - "Disabled", - "Incrementing", - "Alternating 0x55/0xAA", - "All Zeros 0x00", - "All Ones 0xFF", - "Pseudo-random Data", - "User Specified", - "Complex pattern", - "Color box", - "Color bars", -}; +extern const char * const csid_testgen_modes[]; struct csid_format { u32 code;
Hey Hans, Thanks for looking into this. On Tue, 16 Mar 2021 at 10:36, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 15/03/2021 16:59, Robert Foss wrote: > > In order to support Qualcomm ISP hardware architectures that diverge > > from older architectures, the CSID subdevice drivers needs to be refactored > > to better abstract the different ISP hardware architectures. > > > > Signed-off-by: Robert Foss <robert.foss@linaro.org> > > Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org> > > --- > > > > > > Changes since v1: > > - kernel test robot: Add missing include, interrupt.h > > > > Changes since v4: > > - Andrey: Removed whitespace from some includes > > - Andrey: Removed unused enum > > > > Changes since v5: > > - Andrey: Fixed test pattern selection logic > > - Andrey: Align test mode enum values with v4l mode selection return values > > - Andrey: r-b > > - Move Titan 170 test modes to the the Titan 170 commit > > - Fixed test pattern boundary check > > > > Changes since v7: > > - Hans: Fix checkpatch.pl --strict warnings > > > > > > > > drivers/media/platform/qcom/camss/Makefile | 2 + > > .../platform/qcom/camss/camss-csid-4-1.c | 328 ++++++++++ > > .../platform/qcom/camss/camss-csid-4-7.c | 404 ++++++++++++ > > .../media/platform/qcom/camss/camss-csid.c | 608 +----------------- > > .../media/platform/qcom/camss/camss-csid.h | 129 +++- > > 5 files changed, 885 insertions(+), 586 deletions(-) > > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-1.c > > create mode 100644 drivers/media/platform/qcom/camss/camss-csid-4-7.c > > > > <snip> > > > diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h > > index 479ac1f83836..613ef377b051 100644 > > --- a/drivers/media/platform/qcom/camss/camss-csid.h > > +++ b/drivers/media/platform/qcom/camss/camss-csid.h > > @@ -11,6 +11,7 @@ > > #define QC_MSM_CAMSS_CSID_H > > > > #include <linux/clk.h> > > +#include <linux/interrupt.h> > > #include <media/media-entity.h> > > #include <media/v4l2-ctrls.h> > > #include <media/v4l2-device.h> > > @@ -44,18 +45,42 @@ > > #define DATA_TYPE_RAW_16BIT 0x2e > > #define DATA_TYPE_RAW_20BIT 0x2f > > > > -enum csid_payload_mode { > > - CSID_PAYLOAD_MODE_INCREMENTING = 0, > > - CSID_PAYLOAD_MODE_ALTERNATING_55_AA = 1, > > - CSID_PAYLOAD_MODE_ALL_ZEROES = 2, > > - CSID_PAYLOAD_MODE_ALL_ONES = 3, > > - CSID_PAYLOAD_MODE_RANDOM = 4, > > - CSID_PAYLOAD_MODE_USER_SPECIFIED = 5, > > +#define CSID_RESET_TIMEOUT_MS 500 > > + > > +enum csid_testgen_mode { > > + CSID_PAYLOAD_MODE_DISABLED = 0, > > + CSID_PAYLOAD_MODE_INCREMENTING = 1, > > + CSID_PAYLOAD_MODE_ALTERNATING_55_AA = 2, > > + CSID_PAYLOAD_MODE_ALL_ZEROES = 3, > > + CSID_PAYLOAD_MODE_ALL_ONES = 4, > > + CSID_PAYLOAD_MODE_RANDOM = 5, > > + CSID_PAYLOAD_MODE_USER_SPECIFIED = 6, > > + CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN1 = 6, /* excluding disabled */ > > +}; > > + > > +static const char * const csid_testgen_modes[] = { > > + "Disabled", > > + "Incrementing", > > + "Alternating 0x55/0xAA", > > + "All Zeros 0x00", > > + "All Ones 0xFF", > > + "Pseudo-random Data", > > + "User Specified", > > +}; > > This gives this sparse warning: > > 'csid_testgen_modes' defined but not used [-Wunused-const-variable=] Thanks for supplying a patch. I'll merge it into patch 9 & 10. > > This array needs to be moved to camss-csid.c and declared as an extern > here. Also, this menu array needs to be terminated with a NULL, and the > right capitalization needs to be used (first character of each word must > be a capital). This is a suggested patch I made to verify that this solves > this issue, but really both patch 9 and 10 need to be modified. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > --- > drivers/media/platform/qcom/camss/camss-csid.c | 14 ++++++++++++++ > drivers/media/platform/qcom/camss/camss-csid.h | 13 +------------ > 2 files changed, 15 insertions(+), 12 deletions(-) > > diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c > index fb94dc03ccd4..1513b3d47fc2 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.c > +++ b/drivers/media/platform/qcom/camss/camss-csid.c > @@ -27,6 +27,20 @@ > > #define MSM_CSID_NAME "msm_csid" > > +const char * const csid_testgen_modes[] = { > + "Disabled", > + "Incrementing", > + "Alternating 0x55/0xAA", > + "All Zeros 0x00", > + "All Ones 0xFF", > + "Pseudo-Random Data", > + "User Specified", > + "Complex Pattern", > + "Color Box", > + "Color Bars", > + NULL > +}; > + > u32 csid_find_code(u32 *codes, unsigned int ncodes, > unsigned int match_format_idx, u32 match_code) > { > diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h > index c2a025f6846b..81a3704ac0e3 100644 > --- a/drivers/media/platform/qcom/camss/camss-csid.h > +++ b/drivers/media/platform/qcom/camss/camss-csid.h > @@ -62,18 +62,7 @@ enum csid_testgen_mode { > CSID_PAYLOAD_MODE_NUM_SUPPORTED_GEN2 = 9, /* excluding disabled */ > }; > > -static const char * const csid_testgen_modes[] = { > - "Disabled", > - "Incrementing", > - "Alternating 0x55/0xAA", > - "All Zeros 0x00", > - "All Ones 0xFF", > - "Pseudo-random Data", > - "User Specified", > - "Complex pattern", > - "Color box", > - "Color bars", > -}; > +extern const char * const csid_testgen_modes[]; > > struct csid_format { > u32 code; > -- > 2.30.1 > > Regards, > > Hans