[v5,03/10] media: rkisp1: Support devices lacking self path
Commit Message
Some versions of the ISP supported by the rkisp1 driver, such as the ISP
in the i.MX8MP, lack the self path. Support those ISP versions by adding
a self path feature flag, and massage the rest of the driver to support
the lack of a self path.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Tested-by: Adam Ford <aford173@gmail.com>
---
Changes since v3:
- Document the feature bit
- Reorder commit
Changes since v2:
- Simplify rkisp1_path_count()
- Use the rkisp1_has_feature() macro
---
.../platform/rockchip/rkisp1/rkisp1-capture.c | 9 ++++++---
.../platform/rockchip/rkisp1/rkisp1-common.h | 15 +++++++++++++++
.../media/platform/rockchip/rkisp1/rkisp1-dev.c | 9 ++++++---
.../platform/rockchip/rkisp1/rkisp1-resizer.c | 6 ++++--
4 files changed, 31 insertions(+), 8 deletions(-)
Comments
On 06/01/2024 18:02, Paul Elder wrote:
> Some versions of the ISP supported by the rkisp1 driver, such as the ISP
> in the i.MX8MP, lack the self path. Support those ISP versions by adding
> a self path feature flag, and massage the rest of the driver to support
> the lack of a self path.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Tested-by: Adam Ford <aford173@gmail.com>
> ---
> Changes since v3:
>
> - Document the feature bit
> - Reorder commit
>
> Changes since v2:
>
> - Simplify rkisp1_path_count()
> - Use the rkisp1_has_feature() macro
> ---
> .../platform/rockchip/rkisp1/rkisp1-capture.c | 9 ++++++---
> .../platform/rockchip/rkisp1/rkisp1-common.h | 15 +++++++++++++++
> .../media/platform/rockchip/rkisp1/rkisp1-dev.c | 9 ++++++---
> .../platform/rockchip/rkisp1/rkisp1-resizer.c | 6 ++++--
> 4 files changed, 31 insertions(+), 8 deletions(-)
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> index 83a968487f24..ca95f62822fa 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -730,6 +730,7 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
> {
> struct device *dev = ctx;
> struct rkisp1_device *rkisp1 = dev_get_drvdata(dev);
> + unsigned int dev_count = rkisp1_path_count(rkisp1);
> unsigned int i;
> u32 status;
>
> @@ -742,7 +743,7 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
>
> rkisp1_write(rkisp1, RKISP1_CIF_MI_ICR, status);
>
> - for (i = 0; i < ARRAY_SIZE(rkisp1->capture_devs); ++i) {
> + for (i = 0; i < dev_count; ++i) {
> struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
>
> if (!(status & RKISP1_CIF_MI_FRAME(cap)))
> @@ -899,6 +900,7 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
> {
> struct rkisp1_device *rkisp1 = cap->rkisp1;
> struct rkisp1_capture *other = &rkisp1->capture_devs[cap->id ^ 1];
> + bool has_self_path = rkisp1_has_feature(rkisp1, SELF_PATH);
>
> cap->ops->set_data_path(cap);
> cap->ops->config(cap);
> @@ -916,7 +918,7 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
> * This's also required because the second FE maybe corrupt
> * especially when run at 120fps.
> */
> - if (!other->is_streaming) {
> + if (!has_self_path || !other->is_streaming) {
> /* force cfg update */
> rkisp1_write(rkisp1, RKISP1_CIF_MI_INIT,
> RKISP1_CIF_MI_INIT_SOFT_UPD);
> @@ -1509,10 +1511,11 @@ rkisp1_capture_init(struct rkisp1_device *rkisp1, enum rkisp1_stream_id id)
>
> int rkisp1_capture_devs_register(struct rkisp1_device *rkisp1)
> {
> + unsigned int dev_count = rkisp1_path_count(rkisp1);
> unsigned int i;
> int ret;
>
> - for (i = 0; i < ARRAY_SIZE(rkisp1->capture_devs); i++) {
> + for (i = 0; i < dev_count; i++) {
> struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
>
> rkisp1_capture_init(rkisp1, i);
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index 6a811b7ef1b9..f7c251f79aa9 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -111,6 +111,7 @@ enum rkisp1_isp_pad {
> *
> * @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver
> * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path
> + * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path
> *
> * The ISP features are stored in a bitmask in &rkisp1_info.features and allow
> * the driver to implement support for features present in some ISP versions
> @@ -119,6 +120,7 @@ enum rkisp1_isp_pad {
> enum rkisp1_feature {
> RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
> RKISP1_FEATURE_MAIN_STRIDE = BIT(1),
> + RKISP1_FEATURE_SELF_PATH = BIT(2),
> };
>
> #define rkisp1_has_feature(rkisp1, feature) \
> @@ -531,6 +533,19 @@ int rkisp1_cap_enum_mbus_codes(struct rkisp1_capture *cap,
> */
> const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_index(unsigned int index);
>
> +/*
> + * rkisp1_path_count - Return the number of paths supported by the device
> + *
> + * Some devices only have a main path, while other device have both a main path
> + * and a self path. This function returns the number of paths that this device
> + * has, based on the feature flags. It should be used insted of checking
> + * ARRAY_SIZE of capture_devs/resizer_devs.
> + */
> +static inline unsigned int rkisp1_path_count(struct rkisp1_device *rkisp1)
> +{
> + return rkisp1_has_feature(rkisp1, SELF_PATH) ? 2 : 1;
> +}
> +
> /*
> * rkisp1_sd_adjust_crop_rect - adjust a rectangle to fit into another rectangle.
> *
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index 44b753026ba2..106040c4181c 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -358,6 +358,7 @@ static const struct dev_pm_ops rkisp1_pm_ops = {
>
> static int rkisp1_create_links(struct rkisp1_device *rkisp1)
> {
> + unsigned int dev_count = rkisp1_path_count(rkisp1);
> unsigned int i;
> int ret;
>
> @@ -373,7 +374,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
> }
>
> /* create ISP->RSZ->CAP links */
> - for (i = 0; i < 2; i++) {
> + for (i = 0; i < dev_count; i++) {
> struct media_entity *resizer =
> &rkisp1->resizer_devs[i].sd.entity;
> struct media_entity *capture =
> @@ -505,7 +506,8 @@ static const struct rkisp1_info px30_isp_info = {
> .isrs = px30_isp_isrs,
> .isr_size = ARRAY_SIZE(px30_isp_isrs),
> .isp_ver = RKISP1_V12,
> - .features = RKISP1_FEATURE_MIPI_CSI2,
> + .features = RKISP1_FEATURE_MIPI_CSI2
> + | RKISP1_FEATURE_SELF_PATH,
> };
>
> static const char * const rk3399_isp_clks[] = {
> @@ -524,7 +526,8 @@ static const struct rkisp1_info rk3399_isp_info = {
> .isrs = rk3399_isp_isrs,
> .isr_size = ARRAY_SIZE(rk3399_isp_isrs),
> .isp_ver = RKISP1_V10,
> - .features = RKISP1_FEATURE_MIPI_CSI2,
> + .features = RKISP1_FEATURE_MIPI_CSI2
> + | RKISP1_FEATURE_SELF_PATH,
> };
>
> static const struct of_device_id rkisp1_of_match[] = {
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> index a8e377701302..dd77a31e6014 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
> @@ -631,6 +631,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
> struct rkisp1_device *rkisp1 = rsz->rkisp1;
> struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
> enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
> + bool has_self_path = rkisp1_has_feature(rkisp1, SELF_PATH);
> struct v4l2_subdev_state *sd_state;
>
> if (!enable) {
> @@ -639,7 +640,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
> return 0;
> }
>
> - if (other->is_streaming)
> + if (has_self_path && other->is_streaming)
> when = RKISP1_SHADOW_REGS_ASYNC;
>
> sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> @@ -731,10 +732,11 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
>
> int rkisp1_resizer_devs_register(struct rkisp1_device *rkisp1)
> {
> + unsigned int dev_count = rkisp1_path_count(rkisp1);
> unsigned int i;
> int ret;
>
> - for (i = 0; i < ARRAY_SIZE(rkisp1->resizer_devs); i++) {
> + for (i = 0; i < dev_count; i++) {
> struct rkisp1_resizer *rsz = &rkisp1->resizer_devs[i];
>
> rsz->rkisp1 = rkisp1;
@@ -730,6 +730,7 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
{
struct device *dev = ctx;
struct rkisp1_device *rkisp1 = dev_get_drvdata(dev);
+ unsigned int dev_count = rkisp1_path_count(rkisp1);
unsigned int i;
u32 status;
@@ -742,7 +743,7 @@ irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
rkisp1_write(rkisp1, RKISP1_CIF_MI_ICR, status);
- for (i = 0; i < ARRAY_SIZE(rkisp1->capture_devs); ++i) {
+ for (i = 0; i < dev_count; ++i) {
struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
if (!(status & RKISP1_CIF_MI_FRAME(cap)))
@@ -899,6 +900,7 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
{
struct rkisp1_device *rkisp1 = cap->rkisp1;
struct rkisp1_capture *other = &rkisp1->capture_devs[cap->id ^ 1];
+ bool has_self_path = rkisp1_has_feature(rkisp1, SELF_PATH);
cap->ops->set_data_path(cap);
cap->ops->config(cap);
@@ -916,7 +918,7 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
* This's also required because the second FE maybe corrupt
* especially when run at 120fps.
*/
- if (!other->is_streaming) {
+ if (!has_self_path || !other->is_streaming) {
/* force cfg update */
rkisp1_write(rkisp1, RKISP1_CIF_MI_INIT,
RKISP1_CIF_MI_INIT_SOFT_UPD);
@@ -1509,10 +1511,11 @@ rkisp1_capture_init(struct rkisp1_device *rkisp1, enum rkisp1_stream_id id)
int rkisp1_capture_devs_register(struct rkisp1_device *rkisp1)
{
+ unsigned int dev_count = rkisp1_path_count(rkisp1);
unsigned int i;
int ret;
- for (i = 0; i < ARRAY_SIZE(rkisp1->capture_devs); i++) {
+ for (i = 0; i < dev_count; i++) {
struct rkisp1_capture *cap = &rkisp1->capture_devs[i];
rkisp1_capture_init(rkisp1, i);
@@ -111,6 +111,7 @@ enum rkisp1_isp_pad {
*
* @RKISP1_FEATURE_MIPI_CSI2: The ISP has an internal MIPI CSI-2 receiver
* @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path
+ * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path
*
* The ISP features are stored in a bitmask in &rkisp1_info.features and allow
* the driver to implement support for features present in some ISP versions
@@ -119,6 +120,7 @@ enum rkisp1_isp_pad {
enum rkisp1_feature {
RKISP1_FEATURE_MIPI_CSI2 = BIT(0),
RKISP1_FEATURE_MAIN_STRIDE = BIT(1),
+ RKISP1_FEATURE_SELF_PATH = BIT(2),
};
#define rkisp1_has_feature(rkisp1, feature) \
@@ -531,6 +533,19 @@ int rkisp1_cap_enum_mbus_codes(struct rkisp1_capture *cap,
*/
const struct rkisp1_mbus_info *rkisp1_mbus_info_get_by_index(unsigned int index);
+/*
+ * rkisp1_path_count - Return the number of paths supported by the device
+ *
+ * Some devices only have a main path, while other device have both a main path
+ * and a self path. This function returns the number of paths that this device
+ * has, based on the feature flags. It should be used insted of checking
+ * ARRAY_SIZE of capture_devs/resizer_devs.
+ */
+static inline unsigned int rkisp1_path_count(struct rkisp1_device *rkisp1)
+{
+ return rkisp1_has_feature(rkisp1, SELF_PATH) ? 2 : 1;
+}
+
/*
* rkisp1_sd_adjust_crop_rect - adjust a rectangle to fit into another rectangle.
*
@@ -358,6 +358,7 @@ static const struct dev_pm_ops rkisp1_pm_ops = {
static int rkisp1_create_links(struct rkisp1_device *rkisp1)
{
+ unsigned int dev_count = rkisp1_path_count(rkisp1);
unsigned int i;
int ret;
@@ -373,7 +374,7 @@ static int rkisp1_create_links(struct rkisp1_device *rkisp1)
}
/* create ISP->RSZ->CAP links */
- for (i = 0; i < 2; i++) {
+ for (i = 0; i < dev_count; i++) {
struct media_entity *resizer =
&rkisp1->resizer_devs[i].sd.entity;
struct media_entity *capture =
@@ -505,7 +506,8 @@ static const struct rkisp1_info px30_isp_info = {
.isrs = px30_isp_isrs,
.isr_size = ARRAY_SIZE(px30_isp_isrs),
.isp_ver = RKISP1_V12,
- .features = RKISP1_FEATURE_MIPI_CSI2,
+ .features = RKISP1_FEATURE_MIPI_CSI2
+ | RKISP1_FEATURE_SELF_PATH,
};
static const char * const rk3399_isp_clks[] = {
@@ -524,7 +526,8 @@ static const struct rkisp1_info rk3399_isp_info = {
.isrs = rk3399_isp_isrs,
.isr_size = ARRAY_SIZE(rk3399_isp_isrs),
.isp_ver = RKISP1_V10,
- .features = RKISP1_FEATURE_MIPI_CSI2,
+ .features = RKISP1_FEATURE_MIPI_CSI2
+ | RKISP1_FEATURE_SELF_PATH,
};
static const struct of_device_id rkisp1_of_match[] = {
@@ -631,6 +631,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
struct rkisp1_device *rkisp1 = rsz->rkisp1;
struct rkisp1_capture *other = &rkisp1->capture_devs[rsz->id ^ 1];
enum rkisp1_shadow_regs_when when = RKISP1_SHADOW_REGS_SYNC;
+ bool has_self_path = rkisp1_has_feature(rkisp1, SELF_PATH);
struct v4l2_subdev_state *sd_state;
if (!enable) {
@@ -639,7 +640,7 @@ static int rkisp1_rsz_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
- if (other->is_streaming)
+ if (has_self_path && other->is_streaming)
when = RKISP1_SHADOW_REGS_ASYNC;
sd_state = v4l2_subdev_lock_and_get_active_state(sd);
@@ -731,10 +732,11 @@ static int rkisp1_rsz_register(struct rkisp1_resizer *rsz)
int rkisp1_resizer_devs_register(struct rkisp1_device *rkisp1)
{
+ unsigned int dev_count = rkisp1_path_count(rkisp1);
unsigned int i;
int ret;
- for (i = 0; i < ARRAY_SIZE(rkisp1->resizer_devs); i++) {
+ for (i = 0; i < dev_count; i++) {
struct rkisp1_resizer *rsz = &rkisp1->resizer_devs[i];
rsz->rkisp1 = rkisp1;