Re: [PATCH v2 4/5] drm: rcar-du: Split CRTC IRQ and Clock features

From: Laurent Pinchart
Date: Wed Sep 22 2021 - 13:34:59 EST


Hi Kieran,

Thank you for the patch.

On Thu, Sep 02, 2021 at 12:49:06AM +0100, Kieran Bingham wrote:
> Not all platforms require both per-crtc IRQ and per-crtc clock
> management. In preparation for suppporting such platforms, split the
> feature macro to be able to specify both features independently.
>
> The other features are incremented accordingly, to keep the two crtc
> features adjacent.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>

Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

> ---
> v2:
> - New patch
>
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 +--
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 48 +++++++++++++++++---------
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 9 ++---
> 3 files changed, 39 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index a0f837e8243a..5672830ca184 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -1206,7 +1206,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
> int ret;
>
> /* Get the CRTC clock and the optional external clock. */
> - if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
> + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_CLOCK)) {
> sprintf(clk_name, "du.%u", hwindex);
> name = clk_name;
> } else {
> @@ -1272,7 +1272,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int swindex,
> drm_crtc_helper_add(crtc, &crtc_helper_funcs);
>
> /* Register the interrupt handler. */
> - if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
> + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ)) {
> /* The IRQ's are associated with the CRTC (sw)index. */
> irq = platform_get_irq(pdev, swindex);
> irqflags = 0;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 4ac26d08ebb4..8a094d5b9c77 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -36,7 +36,8 @@
>
> static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .channels_mask = BIT(1) | BIT(0),
> @@ -58,7 +59,8 @@ static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
>
> static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .channels_mask = BIT(1) | BIT(0),
> @@ -79,7 +81,8 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
>
> static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .channels_mask = BIT(1) | BIT(0),
> @@ -105,7 +108,8 @@ static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a774a1_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -134,7 +138,8 @@ static const struct rcar_du_device_info rcar_du_r8a774a1_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a774b1_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -163,7 +168,8 @@ static const struct rcar_du_device_info rcar_du_r8a774b1_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a774c0_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE,
> .channels_mask = BIT(1) | BIT(0),
> .routes = {
> @@ -189,7 +195,8 @@ static const struct rcar_du_device_info rcar_du_r8a774c0_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a774e1_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -239,7 +246,8 @@ static const struct rcar_du_device_info rcar_du_r8a7779_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a7790_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .quirks = RCAR_DU_QUIRK_ALIGN_128B,
> @@ -269,7 +277,8 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = {
> /* M2-W (r8a7791) and M2-N (r8a7793) are identical */
> static const struct rcar_du_device_info rcar_du_r8a7791_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .channels_mask = BIT(1) | BIT(0),
> @@ -292,7 +301,8 @@ static const struct rcar_du_device_info rcar_du_r8a7791_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a7792_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .channels_mask = BIT(1) | BIT(0),
> @@ -311,7 +321,8 @@ static const struct rcar_du_device_info rcar_du_r8a7792_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a7794_info = {
> .gen = 2,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> .channels_mask = BIT(1) | BIT(0),
> @@ -333,7 +344,8 @@ static const struct rcar_du_device_info rcar_du_r8a7794_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a7795_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -366,7 +378,8 @@ static const struct rcar_du_device_info rcar_du_r8a7795_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a7796_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -395,7 +408,8 @@ static const struct rcar_du_device_info rcar_du_r8a7796_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a77965_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -424,7 +438,8 @@ static const struct rcar_du_device_info rcar_du_r8a77965_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a77970_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -448,7 +463,8 @@ static const struct rcar_du_device_info rcar_du_r8a77970_info = {
>
> static const struct rcar_du_device_info rcar_du_r8a7799x_info = {
> .gen = 3,
> - .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> + .features = RCAR_DU_FEATURE_CRTC_IRQ
> + | RCAR_DU_FEATURE_CRTC_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE,
> .channels_mask = BIT(1) | BIT(0),
> .routes = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index 02ca2d0e1b55..5fe9152454ff 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -26,10 +26,11 @@ struct drm_bridge;
> struct drm_property;
> struct rcar_du_device;
>
> -#define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK BIT(0) /* Per-CRTC IRQ and clock */
> -#define RCAR_DU_FEATURE_VSP1_SOURCE BIT(1) /* Has inputs from VSP1 */
> -#define RCAR_DU_FEATURE_INTERLACED BIT(2) /* HW supports interlaced */
> -#define RCAR_DU_FEATURE_TVM_SYNC BIT(3) /* Has TV switch/sync modes */
> +#define RCAR_DU_FEATURE_CRTC_IRQ BIT(0) /* Per-CRTC IRQ */
> +#define RCAR_DU_FEATURE_CRTC_CLOCK BIT(1) /* Per-CRTC clock */
> +#define RCAR_DU_FEATURE_VSP1_SOURCE BIT(2) /* Has inputs from VSP1 */
> +#define RCAR_DU_FEATURE_INTERLACED BIT(3) /* HW supports interlaced */
> +#define RCAR_DU_FEATURE_TVM_SYNC BIT(4) /* Has TV switch/sync modes */
>
> #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128 bytes */
>

--
Regards,

Laurent Pinchart