* [PATCH 0/2] media: Add line end IRQ to imx-mipi-csis driver @ 2025-06-06 12:14 Isaac Scott 2025-06-06 12:14 ` [PATCH 1/2] media: platform: Refactor interrupt status registers Isaac Scott 2025-06-06 12:14 ` [PATCH 2/2] media: platform: Add user line interrupt to imx-mipi-csis driver Isaac Scott 0 siblings, 2 replies; 7+ messages in thread From: Isaac Scott @ 2025-06-06 12:14 UTC (permalink / raw) To: laurent.pinchart Cc: kieran.bingham, rmfrfs, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel, Isaac Scott Many boards, such as the NXP i.MX 8M Plus, feature multiple interrupt registers. This series refactors interrupt status register debug handling to make it more intuitive to add other registers such as LINE_END, which has an entire register containing only one interrupt. Previously, the mipi_csi_events[] list contained a debug enable field, and this replaces that with a status_index, which indicates which status register contains the mask for the interrupt. The second patch adds the user line interrupt, which is useful for debugging, as it allows a user to trigger an interrupt after the MIPI CSI receiver has counted a configurable number of lines. This can make it possible to discern the true resolution of the image stream reaching the CSI receiver. It adds an entry to debugfs which lets users choose how many lines are needed to trigger the interrupt, and can be disabled both within and outside streaming by setting the value to 0. Isaac Scott (2): media: platform: Refactor interrupt status registers media: platform: Add user line interrupt to imx-mipi-csis driver drivers/media/platform/nxp/imx-mipi-csis.c | 107 ++++++++++++++------- 1 file changed, 74 insertions(+), 33 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] media: platform: Refactor interrupt status registers 2025-06-06 12:14 [PATCH 0/2] media: Add line end IRQ to imx-mipi-csis driver Isaac Scott @ 2025-06-06 12:14 ` Isaac Scott 2025-06-06 12:34 ` Laurent Pinchart 2025-06-06 13:56 ` Rui Miguel Silva 2025-06-06 12:14 ` [PATCH 2/2] media: platform: Add user line interrupt to imx-mipi-csis driver Isaac Scott 1 sibling, 2 replies; 7+ messages in thread From: Isaac Scott @ 2025-06-06 12:14 UTC (permalink / raw) To: laurent.pinchart Cc: kieran.bingham, rmfrfs, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel, Isaac Scott The NXP i.MX 8 MP CSI-2 receiver features multiple interrupt and debug status sources which span multiple registers. The driver currently supports two interrupt source registers, and attributes the mipi_csis_event event entries to those registers through a boolean debug field that indicate if the event relates to the main interrupt status (false) or debug interrupt status (true) register. To make it easier to add new event fields, replace the debug bool with a 'status index' integer than indicates the index of the corresponding status register. Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> --- drivers/media/platform/nxp/imx-mipi-csis.c | 64 +++++++++++----------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c index d060eadebc7a..bbc549c22aff 100644 --- a/drivers/media/platform/nxp/imx-mipi-csis.c +++ b/drivers/media/platform/nxp/imx-mipi-csis.c @@ -249,7 +249,7 @@ #define MIPI_CSI2_DATA_TYPE_USER(x) (0x30 + (x)) struct mipi_csis_event { - bool debug; + unsigned int status_index; u32 mask; const char * const name; unsigned int counter; @@ -257,30 +257,30 @@ struct mipi_csis_event { static const struct mipi_csis_event mipi_csis_events[] = { /* Errors */ - { false, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO Overflow Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong Configuration Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error" }, - { false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error" }, - { true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type Not Supported" }, - { true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type Ignored" }, - { true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame Size Error" }, - { true, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame" }, - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early Frame End" }, - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early Frame Start" }, + { 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO Overflow Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong Configuration Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error"}, + { 0, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type Not Supported"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type Ignored"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame Size Error"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early Frame End"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early Frame Start"}, /* Non-image data receive events */ - { false, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame" }, - { false, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image data after even frame" }, - { false, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image data before odd frame" }, - { false, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image data after odd frame" }, + { 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame"}, + { 0, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image data after even frame"}, + { 0, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image data before odd frame"}, + { 0, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image data after odd frame"}, /* Frame start/end */ - { false, MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start" }, - { false, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End" }, - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge" }, - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge" }, + { 0, MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start"}, + { 0, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge"}, + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge"}, }; #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events) @@ -765,32 +765,30 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) struct mipi_csis_device *csis = dev_id; unsigned long flags; unsigned int i; - u32 status; - u32 dbg_status; + u32 status[2]; - status = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); - dbg_status = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); + status[0] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); + status[1] = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); spin_lock_irqsave(&csis->slock, flags); /* Update the event/error counters */ - if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) { + if ((status[0] & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) { for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) { struct mipi_csis_event *event = &csis->events[i]; - if ((!event->debug && (status & event->mask)) || - (event->debug && (dbg_status & event->mask))) + if (status[event->status_index] & event->mask) event->counter++; } } - if (status & MIPI_CSIS_INT_SRC_FRAME_START) + if (status[0] & MIPI_CSIS_INT_SRC_FRAME_START) mipi_csis_queue_event_sof(csis); spin_unlock_irqrestore(&csis->slock, flags); - mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status); - mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, dbg_status); + mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status[0]); + mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, status[1]); return IRQ_HANDLED; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: platform: Refactor interrupt status registers 2025-06-06 12:14 ` [PATCH 1/2] media: platform: Refactor interrupt status registers Isaac Scott @ 2025-06-06 12:34 ` Laurent Pinchart 2025-06-06 13:56 ` Rui Miguel Silva 1 sibling, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2025-06-06 12:34 UTC (permalink / raw) To: Isaac Scott Cc: kieran.bingham, rmfrfs, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel Hi Isaac, Thank you for the patch. On Fri, Jun 06, 2025 at 01:14:02PM +0100, Isaac Scott wrote: > The NXP i.MX 8 MP CSI-2 receiver features multiple interrupt and debug > status sources which span multiple registers. The driver currently > supports two interrupt source registers, and attributes the > mipi_csis_event event entries to those registers through a boolean debug > field that indicate if the event relates to the main interrupt status > (false) or debug interrupt status (true) register. To make it easier to > add new event fields, replace the debug bool with a 'status index' > integer than indicates the index of the corresponding status register. > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/platform/nxp/imx-mipi-csis.c | 64 +++++++++++----------- > 1 file changed, 31 insertions(+), 33 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c > index d060eadebc7a..bbc549c22aff 100644 > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > @@ -249,7 +249,7 @@ > #define MIPI_CSI2_DATA_TYPE_USER(x) (0x30 + (x)) > > struct mipi_csis_event { > - bool debug; > + unsigned int status_index; > u32 mask; > const char * const name; > unsigned int counter; > @@ -257,30 +257,30 @@ struct mipi_csis_event { > > static const struct mipi_csis_event mipi_csis_events[] = { > /* Errors */ > - { false, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO Overflow Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong Configuration Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type Not Supported" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type Ignored" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame Size Error" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early Frame End" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early Frame Start" }, > + { 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO Overflow Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong Configuration Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type Not Supported"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type Ignored"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame Size Error"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early Frame End"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early Frame Start"}, > /* Non-image data receive events */ > - { false, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame" }, > - { false, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image data after even frame" }, > - { false, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image data before odd frame" }, > - { false, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image data after odd frame" }, > + { 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame"}, > + { 0, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image data after even frame"}, > + { 0, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image data before odd frame"}, > + { 0, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image data after odd frame"}, > /* Frame start/end */ > - { false, MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start" }, > - { false, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge" }, > + { 0, MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start"}, > + { 0, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge"}, > }; > > #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events) > @@ -765,32 +765,30 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > struct mipi_csis_device *csis = dev_id; > unsigned long flags; > unsigned int i; > - u32 status; > - u32 dbg_status; > + u32 status[2]; > > - status = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > - dbg_status = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > + status[0] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > + status[1] = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > > spin_lock_irqsave(&csis->slock, flags); > > /* Update the event/error counters */ > - if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) { > + if ((status[0] & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) { > for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) { > struct mipi_csis_event *event = &csis->events[i]; > > - if ((!event->debug && (status & event->mask)) || > - (event->debug && (dbg_status & event->mask))) > + if (status[event->status_index] & event->mask) > event->counter++; > } > } > > - if (status & MIPI_CSIS_INT_SRC_FRAME_START) > + if (status[0] & MIPI_CSIS_INT_SRC_FRAME_START) > mipi_csis_queue_event_sof(csis); > > spin_unlock_irqrestore(&csis->slock, flags); > > - mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status); > - mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, dbg_status); > + mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status[0]); > + mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, status[1]); > > return IRQ_HANDLED; > } -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: platform: Refactor interrupt status registers 2025-06-06 12:14 ` [PATCH 1/2] media: platform: Refactor interrupt status registers Isaac Scott 2025-06-06 12:34 ` Laurent Pinchart @ 2025-06-06 13:56 ` Rui Miguel Silva 2025-06-06 15:47 ` Isaac Scott 1 sibling, 1 reply; 7+ messages in thread From: Rui Miguel Silva @ 2025-06-06 13:56 UTC (permalink / raw) To: Isaac Scott, laurent.pinchart Cc: kieran.bingham, rmfrfs, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel Hey Isaac, Thanks for the patch. On Fri Jun 6, 2025 at 1:14 PM WEST, Isaac Scott wrote: > The NXP i.MX 8 MP CSI-2 receiver features multiple interrupt and debug > status sources which span multiple registers. The driver currently > supports two interrupt source registers, and attributes the > mipi_csis_event event entries to those registers through a boolean debug > field that indicate if the event relates to the main interrupt status > (false) or debug interrupt status (true) register. To make it easier to > add new event fields, replace the debug bool with a 'status index' > integer than indicates the index of the corresponding status register. > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > --- > drivers/media/platform/nxp/imx-mipi-csis.c | 64 +++++++++++----------- > 1 file changed, 31 insertions(+), 33 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c > index d060eadebc7a..bbc549c22aff 100644 > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > @@ -249,7 +249,7 @@ > #define MIPI_CSI2_DATA_TYPE_USER(x) (0x30 + (x)) > > struct mipi_csis_event { > - bool debug; > + unsigned int status_index; > u32 mask; > const char * const name; > unsigned int counter; > @@ -257,30 +257,30 @@ struct mipi_csis_event { > > static const struct mipi_csis_event mipi_csis_events[] = { > /* Errors */ > - { false, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO Overflow Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong Configuration Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error" }, > - { false, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type Not Supported" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type Ignored" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame Size Error" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early Frame End" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early Frame Start" }, > + { 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT Error"}, Maybe instead of 0,1,2 (magic indexes)... we could give a meaningful index enums names, don't know, like: main, debug, user??? or something that you think is better. Cheers, Rui > + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO Overflow Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong Configuration Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error"}, > + { 0, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type Not Supported"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type Ignored"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame Size Error"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early Frame End"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early Frame Start"}, > /* Non-image data receive events */ > - { false, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame" }, > - { false, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image data after even frame" }, > - { false, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image data before odd frame" }, > - { false, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image data after odd frame" }, > + { 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame"}, > + { 0, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image data after even frame"}, > + { 0, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image data before odd frame"}, > + { 0, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image data after odd frame"}, > /* Frame start/end */ > - { false, MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start" }, > - { false, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge" }, > - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge" }, > + { 0, MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start"}, > + { 0, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge"}, > + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge"}, > }; > > #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events) > @@ -765,32 +765,30 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > struct mipi_csis_device *csis = dev_id; > unsigned long flags; > unsigned int i; > - u32 status; > - u32 dbg_status; > + u32 status[2]; > > - status = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > - dbg_status = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > + status[0] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > + status[1] = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > > spin_lock_irqsave(&csis->slock, flags); > > /* Update the event/error counters */ > - if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) { > + if ((status[0] & MIPI_CSIS_INT_SRC_ERRORS) || csis->debug.enable) { > for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) { > struct mipi_csis_event *event = &csis->events[i]; > > - if ((!event->debug && (status & event->mask)) || > - (event->debug && (dbg_status & event->mask))) > + if (status[event->status_index] & event->mask) > event->counter++; > } > } > > - if (status & MIPI_CSIS_INT_SRC_FRAME_START) > + if (status[0] & MIPI_CSIS_INT_SRC_FRAME_START) > mipi_csis_queue_event_sof(csis); > > spin_unlock_irqrestore(&csis->slock, flags); > > - mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status); > - mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, dbg_status); > + mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status[0]); > + mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, status[1]); > > return IRQ_HANDLED; > } > -- > 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: platform: Refactor interrupt status registers 2025-06-06 13:56 ` Rui Miguel Silva @ 2025-06-06 15:47 ` Isaac Scott 0 siblings, 0 replies; 7+ messages in thread From: Isaac Scott @ 2025-06-06 15:47 UTC (permalink / raw) To: Rui Miguel Silva, laurent.pinchart Cc: kieran.bingham, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel Hi Rui, On Fri, 2025-06-06 at 14:56 +0100, Rui Miguel Silva wrote: > Hey Isaac, > Thanks for the patch. > > On Fri Jun 6, 2025 at 1:14 PM WEST, Isaac Scott wrote: > > > The NXP i.MX 8 MP CSI-2 receiver features multiple interrupt and > > debug > > status sources which span multiple registers. The driver currently > > supports two interrupt source registers, and attributes the > > mipi_csis_event event entries to those registers through a boolean > > debug > > field that indicate if the event relates to the main interrupt > > status > > (false) or debug interrupt status (true) register. To make it > > easier to > > add new event fields, replace the debug bool with a 'status index' > > integer than indicates the index of the corresponding status > > register. > > > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > > --- > > drivers/media/platform/nxp/imx-mipi-csis.c | 64 +++++++++++------- > > ---- > > 1 file changed, 31 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c > > b/drivers/media/platform/nxp/imx-mipi-csis.c > > index d060eadebc7a..bbc549c22aff 100644 > > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > > @@ -249,7 +249,7 @@ > > #define MIPI_CSI2_DATA_TYPE_USER(x) (0x30 + (x)) > > > > struct mipi_csis_event { > > - bool debug; > > + unsigned int status_index; > > u32 mask; > > const char * const name; > > unsigned int counter; > > @@ -257,30 +257,30 @@ struct mipi_csis_event { > > > > static const struct mipi_csis_event mipi_csis_events[] = { > > /* Errors */ > > - { false, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT > > Error" }, > > - { false, > > MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost Frame Start Error" }, > > - { false, > > MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost Frame End Error" }, > > - { false, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO > > Overflow Error" }, > > - { false, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong > > Configuration Error" }, > > - { false, MIPI_CSIS_INT_SRC_ERR_ECC, "ECC > > Error" }, > > - { false, MIPI_CSIS_INT_SRC_ERR_CRC, "CRC > > Error" }, > > - { false, > > MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown Error" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type > > Not Supported" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type > > Ignored" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame > > Size Error" }, > > - { true, > > MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated Frame" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early > > Frame End" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early > > Frame Start" }, > > + { 0, MIPI_CSIS_INT_SRC_ERR_SOT_HS, "SOT > > Error"}, > > Maybe instead of 0,1,2 (magic indexes)... we could give a meaningful > index > enums names, don't know, like: main, debug, user??? or something that > you think is better. Thanks for the review! I have updated v2 to include an enum instead of magic numbers. Best wishes, Isaac > > Cheers, > Rui > > > + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FS, "Lost > > Frame Start Error"}, > > + { 0, MIPI_CSIS_INT_SRC_ERR_LOST_FE, "Lost > > Frame End Error"}, > > + { 0, MIPI_CSIS_INT_SRC_ERR_OVER, "FIFO > > Overflow Error"}, > > + { 0, MIPI_CSIS_INT_SRC_ERR_WRONG_CFG, "Wrong > > Configuration Error"}, > > + { 0, > > MIPI_CSIS_INT_SRC_ERR_ECC, "ECC Error"}, > > + { 0, > > MIPI_CSIS_INT_SRC_ERR_CRC, "CRC Error"}, > > + { 0, MIPI_CSIS_INT_SRC_ERR_UNKNOWN, "Unknown > > Error"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_NOT_SUPPORT, "Data Type > > Not Supported"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_DT_IGNORE, "Data Type > > Ignored"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_ERR_FRAME_SIZE, "Frame > > Size Error"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_TRUNCATED_FRAME, "Truncated > > Frame"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FE, "Early > > Frame End"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_EARLY_FS, "Early > > Frame Start"}, > > /* Non-image data receive events */ > > - { false, > > MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image data before even frame" }, > > - { false, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image > > data after even frame" }, > > - { false, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image > > data before odd frame" }, > > - { false, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image > > data after odd frame" }, > > + { 0, MIPI_CSIS_INT_SRC_EVEN_BEFORE, "Non-image > > data before even frame"}, > > + { 0, MIPI_CSIS_INT_SRC_EVEN_AFTER, "Non-image > > data after even frame"}, > > + { 0, MIPI_CSIS_INT_SRC_ODD_BEFORE, "Non-image > > data before odd frame"}, > > + { 0, MIPI_CSIS_INT_SRC_ODD_AFTER, "Non-image > > data after odd frame"}, > > /* Frame start/end */ > > - { false, > > MIPI_CSIS_INT_SRC_FRAME_START, "Frame Start" }, > > - { false, MIPI_CSIS_INT_SRC_FRAME_END, "Frame > > End" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC > > Falling Edge" }, > > - { true, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC > > Rising Edge" }, > > + { 0, MIPI_CSIS_INT_SRC_FRAME_START, "Frame > > Start"}, > > + { 0, MIPI_CSIS_INT_SRC_FRAME_END, "Frame > > End"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC > > Falling Edge"}, > > + { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC > > Rising Edge"}, > > }; > > > > #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events) > > @@ -765,32 +765,30 @@ static irqreturn_t mipi_csis_irq_handler(int > > irq, void *dev_id) > > struct mipi_csis_device *csis = dev_id; > > unsigned long flags; > > unsigned int i; > > - u32 status; > > - u32 dbg_status; > > + u32 status[2]; > > > > - status = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > > - dbg_status = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > > + status[0] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > > + status[1] = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > > > > spin_lock_irqsave(&csis->slock, flags); > > > > /* Update the event/error counters */ > > - if ((status & MIPI_CSIS_INT_SRC_ERRORS) || csis- > > >debug.enable) { > > + if ((status[0] & MIPI_CSIS_INT_SRC_ERRORS) || csis- > > >debug.enable) { > > for (i = 0; i < MIPI_CSIS_NUM_EVENTS; i++) { > > struct mipi_csis_event *event = &csis- > > >events[i]; > > > > - if ((!event->debug && (status & event- > > >mask)) || > > - (event->debug && (dbg_status & event- > > >mask))) > > + if (status[event->status_index] & event- > > >mask) > > event->counter++; > > } > > } > > > > - if (status & MIPI_CSIS_INT_SRC_FRAME_START) > > + if (status[0] & MIPI_CSIS_INT_SRC_FRAME_START) > > mipi_csis_queue_event_sof(csis); > > > > spin_unlock_irqrestore(&csis->slock, flags); > > > > - mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status); > > - mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, dbg_status); > > + mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status[0]); > > + mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, status[1]); > > > > return IRQ_HANDLED; > > } > > -- > > 2.43.0 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: platform: Add user line interrupt to imx-mipi-csis driver 2025-06-06 12:14 [PATCH 0/2] media: Add line end IRQ to imx-mipi-csis driver Isaac Scott 2025-06-06 12:14 ` [PATCH 1/2] media: platform: Refactor interrupt status registers Isaac Scott @ 2025-06-06 12:14 ` Isaac Scott 2025-06-06 12:40 ` Laurent Pinchart 1 sibling, 1 reply; 7+ messages in thread From: Isaac Scott @ 2025-06-06 12:14 UTC (permalink / raw) To: laurent.pinchart Cc: kieran.bingham, rmfrfs, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel, Isaac Scott The NXP i.MX 8M Plus features an interrupt that triggers after the MIPI CSI receiver counts a user-configurable number of lines. This is useful for debugging, as it allows users to check if the amount of lines per frame equals what they are expecting. Add support for this interrupt in the driver, and an entry into debugfs to allow the user to configure whether the interrupt is enabled, as well as the number of lines after which to trigger the interrupt. This debugfs control can be altered while a stream is in progress, with 0 disabling the interrupt and >0 setting a new desired line count. Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> --- drivers/media/platform/nxp/imx-mipi-csis.c | 45 +++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c index bbc549c22aff..0e6bc3c87be4 100644 --- a/drivers/media/platform/nxp/imx-mipi-csis.c +++ b/drivers/media/platform/nxp/imx-mipi-csis.c @@ -88,6 +88,10 @@ #define MIPI_CSIS_INT_MSK_ERR_CRC BIT(1) #define MIPI_CSIS_INT_MSK_ERR_UNKNOWN BIT(0) +/* CSIS Interrupt mask 1 */ +#define MIPI_CSIS_INT_MSK_1 0x18 +#define MIPI_CSIS_INT_MSK_1_LINE_END BIT(0) + /* CSIS Interrupt source */ #define MIPI_CSIS_INT_SRC 0x14 #define MIPI_CSIS_INT_SRC_EVEN_BEFORE BIT(31) @@ -109,6 +113,10 @@ #define MIPI_CSIS_INT_SRC_ERR_UNKNOWN BIT(0) #define MIPI_CSIS_INT_SRC_ERRORS 0xfffff +/* CSIS Interrupt source 1 */ +#define MIPI_CSIS_INT_SRC_1 0x1c +#define MIPI_CSIS_INT_SRC_1_LINE_END BIT(0) + /* D-PHY status control */ #define MIPI_CSIS_DPHY_STATUS 0x20 #define MIPI_CSIS_DPHY_STATUS_ULPS_DAT BIT(8) @@ -221,6 +229,7 @@ #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE BIT(0) #define MIPI_CSIS_FRAME_COUNTER_CH(n) (0x0100 + (n) * 4) +#define MIPI_CSIS_LINE_INTERRUPT_RATIO(n) (0x0110 + (n) * 4) /* Non-image packet data buffers */ #define MIPI_CSIS_PKTDATA_ODD 0x2000 @@ -281,6 +290,8 @@ static const struct mipi_csis_event mipi_csis_events[] = { { 0, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End"}, { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge"}, { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge"}, + /* User Line interrupt */ + { 2, MIPI_CSIS_INT_SRC_1_LINE_END, "Line End"} }; #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events) @@ -333,11 +344,14 @@ struct mipi_csis_device { spinlock_t slock; /* Protect events */ struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS]; + struct dentry *debugfs_root; struct { bool enable; u32 hs_settle; u32 clk_settle; + u32 int_line; + u32 last_int_line; } debug; }; @@ -686,6 +700,15 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis, MIPI_CSIS_DPHY_BCTRL_L_B_DPHYCTRL(20000000)); mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_H, 0); + if (csis->debug.int_line > 0) + mipi_csis_write(csis, MIPI_CSIS_LINE_INTERRUPT_RATIO(0), + csis->debug.int_line - 1); + + mipi_csis_write(csis, MIPI_CSIS_INT_MSK_1, + csis->debug.int_line ? + MIPI_CSIS_INT_MSK_1_LINE_END : 0); + csis->debug.last_int_line = csis->debug.int_line; + /* Update the shadow register. */ val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, @@ -765,10 +788,12 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) struct mipi_csis_device *csis = dev_id; unsigned long flags; unsigned int i; - u32 status[2]; + u32 int_lines; + u32 status[3]; status[0] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); status[1] = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); + status[2] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC_1); spin_lock_irqsave(&csis->slock, flags); @@ -785,10 +810,25 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) if (status[0] & MIPI_CSIS_INT_SRC_FRAME_START) mipi_csis_queue_event_sof(csis); + int_lines = READ_ONCE(csis->debug.int_line); + if (int_lines != csis->debug.last_int_line) { + if (int_lines > 0) + mipi_csis_write(csis, MIPI_CSIS_LINE_INTERRUPT_RATIO(0), + max(int_lines, 1U) - 1); + else + mipi_csis_write(csis, + MIPI_CSIS_LINE_INTERRUPT_RATIO(0), 0); + + csis->debug.last_int_line = int_lines; + mipi_csis_write(csis, MIPI_CSIS_INT_MSK_1, + int_lines ? MIPI_CSIS_INT_MSK_1_LINE_END : 0); + } + spin_unlock_irqrestore(&csis->slock, flags); mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status[0]); mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, status[1]); + mipi_csis_write(csis, MIPI_CSIS_INT_SRC_1, status[2]); return IRQ_HANDLED; } @@ -928,6 +968,7 @@ static void mipi_csis_debugfs_init(struct mipi_csis_device *csis) { csis->debug.hs_settle = UINT_MAX; csis->debug.clk_settle = UINT_MAX; + csis->debug.int_line = 0; csis->debugfs_root = debugfs_create_dir(dev_name(csis->dev), NULL); @@ -939,6 +980,8 @@ static void mipi_csis_debugfs_init(struct mipi_csis_device *csis) &csis->debug.clk_settle); debugfs_create_u32("ths_settle", 0600, csis->debugfs_root, &csis->debug.hs_settle); + debugfs_create_u32("int_line_0", 0600, csis->debugfs_root, + &csis->debug.int_line); } static void mipi_csis_debugfs_exit(struct mipi_csis_device *csis) -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: platform: Add user line interrupt to imx-mipi-csis driver 2025-06-06 12:14 ` [PATCH 2/2] media: platform: Add user line interrupt to imx-mipi-csis driver Isaac Scott @ 2025-06-06 12:40 ` Laurent Pinchart 0 siblings, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2025-06-06 12:40 UTC (permalink / raw) To: Isaac Scott Cc: kieran.bingham, rmfrfs, martink, kernel, mchehab, shawnguo, s.hauer, kernel, festevam, linux-media, imx, linux-arm-kernel, linux-kernel Hi Isaac, Thank you for the patch. On Fri, Jun 06, 2025 at 01:14:03PM +0100, Isaac Scott wrote: > The NXP i.MX 8M Plus features an interrupt that triggers after the MIPI > CSI receiver counts a user-configurable number of lines. This is useful > for debugging, as it allows users to check if the amount of lines per > frame equals what they are expecting. > > Add support for this interrupt in the driver, and an entry into debugfs to > allow the user to configure whether the interrupt is enabled, as well as > the number of lines after which to trigger the interrupt. > > This debugfs control can be altered while a stream is in progress, with > 0 disabling the interrupt and >0 setting a new desired line count. > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > --- > drivers/media/platform/nxp/imx-mipi-csis.c | 45 +++++++++++++++++++++- > 1 file changed, 44 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/nxp/imx-mipi-csis.c b/drivers/media/platform/nxp/imx-mipi-csis.c > index bbc549c22aff..0e6bc3c87be4 100644 > --- a/drivers/media/platform/nxp/imx-mipi-csis.c > +++ b/drivers/media/platform/nxp/imx-mipi-csis.c > @@ -88,6 +88,10 @@ > #define MIPI_CSIS_INT_MSK_ERR_CRC BIT(1) > #define MIPI_CSIS_INT_MSK_ERR_UNKNOWN BIT(0) > > +/* CSIS Interrupt mask 1 */ > +#define MIPI_CSIS_INT_MSK_1 0x18 > +#define MIPI_CSIS_INT_MSK_1_LINE_END BIT(0) > + > /* CSIS Interrupt source */ > #define MIPI_CSIS_INT_SRC 0x14 > #define MIPI_CSIS_INT_SRC_EVEN_BEFORE BIT(31) > @@ -109,6 +113,10 @@ > #define MIPI_CSIS_INT_SRC_ERR_UNKNOWN BIT(0) > #define MIPI_CSIS_INT_SRC_ERRORS 0xfffff > > +/* CSIS Interrupt source 1 */ > +#define MIPI_CSIS_INT_SRC_1 0x1c > +#define MIPI_CSIS_INT_SRC_1_LINE_END BIT(0) > + > /* D-PHY status control */ > #define MIPI_CSIS_DPHY_STATUS 0x20 > #define MIPI_CSIS_DPHY_STATUS_ULPS_DAT BIT(8) > @@ -221,6 +229,7 @@ > #define MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE BIT(0) > > #define MIPI_CSIS_FRAME_COUNTER_CH(n) (0x0100 + (n) * 4) > +#define MIPI_CSIS_LINE_INTERRUPT_RATIO(n) (0x0110 + (n) * 4) > > /* Non-image packet data buffers */ > #define MIPI_CSIS_PKTDATA_ODD 0x2000 > @@ -281,6 +290,8 @@ static const struct mipi_csis_event mipi_csis_events[] = { > { 0, MIPI_CSIS_INT_SRC_FRAME_END, "Frame End"}, > { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_FALL, "VSYNC Falling Edge"}, > { 1, MIPI_CSIS_DBG_INTR_SRC_CAM_VSYNC_RISE, "VSYNC Rising Edge"}, > + /* User Line interrupt */ > + { 2, MIPI_CSIS_INT_SRC_1_LINE_END, "Line End"} > }; > > #define MIPI_CSIS_NUM_EVENTS ARRAY_SIZE(mipi_csis_events) > @@ -333,11 +344,14 @@ struct mipi_csis_device { > > spinlock_t slock; /* Protect events */ > struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS]; > + > struct dentry *debugfs_root; > struct { > bool enable; > u32 hs_settle; > u32 clk_settle; > + u32 int_line; > + u32 last_int_line; > } debug; > }; > > @@ -686,6 +700,15 @@ static void mipi_csis_set_params(struct mipi_csis_device *csis, > MIPI_CSIS_DPHY_BCTRL_L_B_DPHYCTRL(20000000)); > mipi_csis_write(csis, MIPI_CSIS_DPHY_BCTRL_H, 0); > > + if (csis->debug.int_line > 0) > + mipi_csis_write(csis, MIPI_CSIS_LINE_INTERRUPT_RATIO(0), > + csis->debug.int_line - 1); Should this become mipi_csis_write(csis, MIPI_CSIS_LINE_INTERRUPT_RATIO(0), max(csis->debug.int_line, 1U) - 1); to write MIPI_CSIS_LINE_INTERRUPT_RATIO to 0 when int_lines == 0, like below ? > + > + mipi_csis_write(csis, MIPI_CSIS_INT_MSK_1, > + csis->debug.int_line ? > + MIPI_CSIS_INT_MSK_1_LINE_END : 0); You should also read csis->debug.int_line into a local variable like in the interrupt handler to avoid races. And you also need to clear MIPI_CSIS_INT_MSK_1 in mipi_csis_enable_interrupts() (when on == false). > + csis->debug.last_int_line = csis->debug.int_line; > + > /* Update the shadow register. */ > val = mipi_csis_read(csis, MIPI_CSIS_CMN_CTRL); > mipi_csis_write(csis, MIPI_CSIS_CMN_CTRL, > @@ -765,10 +788,12 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > struct mipi_csis_device *csis = dev_id; > unsigned long flags; > unsigned int i; > - u32 status[2]; > + u32 int_lines; > + u32 status[3]; > > status[0] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC); > status[1] = mipi_csis_read(csis, MIPI_CSIS_DBG_INTR_SRC); > + status[2] = mipi_csis_read(csis, MIPI_CSIS_INT_SRC_1); > > spin_lock_irqsave(&csis->slock, flags); > > @@ -785,10 +810,25 @@ static irqreturn_t mipi_csis_irq_handler(int irq, void *dev_id) > if (status[0] & MIPI_CSIS_INT_SRC_FRAME_START) > mipi_csis_queue_event_sof(csis); > > + int_lines = READ_ONCE(csis->debug.int_line); > + if (int_lines != csis->debug.last_int_line) { > + if (int_lines > 0) > + mipi_csis_write(csis, MIPI_CSIS_LINE_INTERRUPT_RATIO(0), > + max(int_lines, 1U) - 1); > + else > + mipi_csis_write(csis, > + MIPI_CSIS_LINE_INTERRUPT_RATIO(0), 0); The whole can be replaced with mipi_csis_write(csis, MIPI_CSIS_LINE_INTERRUPT_RATIO(0), max(int_lines, 1U) - 1); > + > + csis->debug.last_int_line = int_lines; > + mipi_csis_write(csis, MIPI_CSIS_INT_MSK_1, > + int_lines ? MIPI_CSIS_INT_MSK_1_LINE_END : 0); > + } > + I don't think you need to do any of this with the spinlock held as the spinlock covers the events field only, which you don't touch here. You can move the code after spin_unlock_irqrestore(). > spin_unlock_irqrestore(&csis->slock, flags); > > mipi_csis_write(csis, MIPI_CSIS_INT_SRC, status[0]); > mipi_csis_write(csis, MIPI_CSIS_DBG_INTR_SRC, status[1]); > + mipi_csis_write(csis, MIPI_CSIS_INT_SRC_1, status[2]); > > return IRQ_HANDLED; > } > @@ -928,6 +968,7 @@ static void mipi_csis_debugfs_init(struct mipi_csis_device *csis) > { > csis->debug.hs_settle = UINT_MAX; > csis->debug.clk_settle = UINT_MAX; > + csis->debug.int_line = 0; > > csis->debugfs_root = debugfs_create_dir(dev_name(csis->dev), NULL); > > @@ -939,6 +980,8 @@ static void mipi_csis_debugfs_init(struct mipi_csis_device *csis) > &csis->debug.clk_settle); > debugfs_create_u32("ths_settle", 0600, csis->debugfs_root, > &csis->debug.hs_settle); > + debugfs_create_u32("int_line_0", 0600, csis->debugfs_root, > + &csis->debug.int_line); > } > > static void mipi_csis_debugfs_exit(struct mipi_csis_device *csis) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-06 15:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-06 12:14 [PATCH 0/2] media: Add line end IRQ to imx-mipi-csis driver Isaac Scott 2025-06-06 12:14 ` [PATCH 1/2] media: platform: Refactor interrupt status registers Isaac Scott 2025-06-06 12:34 ` Laurent Pinchart 2025-06-06 13:56 ` Rui Miguel Silva 2025-06-06 15:47 ` Isaac Scott 2025-06-06 12:14 ` [PATCH 2/2] media: platform: Add user line interrupt to imx-mipi-csis driver Isaac Scott 2025-06-06 12:40 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).