Re: [PATCH 17/64] drm/vc4: crtc: Move debugfs_name to crtc_data
On Thu, 16 Jun 2022 at 10:41, Maxime Ripard wrote: > > Hi Dave, > > On Tue, Jun 14, 2022 at 04:57:45PM +0100, Dave Stevenson wrote: > > On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote: > > > > > > All the CRTCs, including the TXP, have a debugfs file and name so we can > > > consolidate it into vc4_crtc_data. > > > > > > Signed-off-by: Maxime Ripard > > > > Reviewed-by: Dave Stevenson > > > > I was sort of expecting the vc4_debugfs_add_regset32 call to move to > > vc4_crtc_init so that it would be a common call for crtcs and txp, but > > that doesn't appear to happen in this set. The debugfs_name for the > > txp is therefore actually unused. > > As of this patch, you're right > > This is later changed by some other patch though: > https://lore.kernel.org/all/20220610092924.754942-60-max...@cerno.tech/ Indeed, and that cleans it all up. Perfect. Dave
Re: [PATCH 17/64] drm/vc4: crtc: Move debugfs_name to crtc_data
Hi Dave, On Tue, Jun 14, 2022 at 04:57:45PM +0100, Dave Stevenson wrote: > On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote: > > > > All the CRTCs, including the TXP, have a debugfs file and name so we can > > consolidate it into vc4_crtc_data. > > > > Signed-off-by: Maxime Ripard > > Reviewed-by: Dave Stevenson > > I was sort of expecting the vc4_debugfs_add_regset32 call to move to > vc4_crtc_init so that it would be a common call for crtcs and txp, but > that doesn't appear to happen in this set. The debugfs_name for the > txp is therefore actually unused. As of this patch, you're right This is later changed by some other patch though: https://lore.kernel.org/all/20220610092924.754942-60-max...@cerno.tech/ Maxime signature.asc Description: PGP signature
Re: [PATCH 17/64] drm/vc4: crtc: Move debugfs_name to crtc_data
On Fri, 10 Jun 2022 at 10:30, Maxime Ripard wrote:
>
> All the CRTCs, including the TXP, have a debugfs file and name so we can
> consolidate it into vc4_crtc_data.
>
> Signed-off-by: Maxime Ripard
Reviewed-by: Dave Stevenson
I was sort of expecting the vc4_debugfs_add_regset32 call to move to
vc4_crtc_init so that it would be a common call for crtcs and txp, but
that doesn't appear to happen in this set. The debugfs_name for the
txp is therefore actually unused.
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 18 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 4 ++--
> drivers/gpu/drm/vc4/vc4_txp.c | 1 +
> 3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 7163f924b48b..1f7e987e68aa 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -978,10 +978,10 @@ static const struct drm_crtc_helper_funcs
> vc4_crtc_helper_funcs = {
>
> static const struct vc4_pv_data bcm2835_pv0_data = {
> .base = {
> + .debugfs_name = "crtc0_regs",
> .hvs_available_channels = BIT(0),
> .hvs_output = 0,
> },
> - .debugfs_name = "crtc0_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 1,
> .encoder_types = {
> @@ -992,10 +992,10 @@ static const struct vc4_pv_data bcm2835_pv0_data = {
>
> static const struct vc4_pv_data bcm2835_pv1_data = {
> .base = {
> + .debugfs_name = "crtc1_regs",
> .hvs_available_channels = BIT(2),
> .hvs_output = 2,
> },
> - .debugfs_name = "crtc1_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 1,
> .encoder_types = {
> @@ -1006,10 +1006,10 @@ static const struct vc4_pv_data bcm2835_pv1_data = {
>
> static const struct vc4_pv_data bcm2835_pv2_data = {
> .base = {
> + .debugfs_name = "crtc2_regs",
> .hvs_available_channels = BIT(1),
> .hvs_output = 1,
> },
> - .debugfs_name = "crtc2_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 1,
> .encoder_types = {
> @@ -1020,10 +1020,10 @@ static const struct vc4_pv_data bcm2835_pv2_data = {
>
> static const struct vc4_pv_data bcm2711_pv0_data = {
> .base = {
> + .debugfs_name = "crtc0_regs",
> .hvs_available_channels = BIT(0),
> .hvs_output = 0,
> },
> - .debugfs_name = "crtc0_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 1,
> .encoder_types = {
> @@ -1034,10 +1034,10 @@ static const struct vc4_pv_data bcm2711_pv0_data = {
>
> static const struct vc4_pv_data bcm2711_pv1_data = {
> .base = {
> + .debugfs_name = "crtc1_regs",
> .hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
> .hvs_output = 3,
> },
> - .debugfs_name = "crtc1_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 1,
> .encoder_types = {
> @@ -1048,10 +1048,10 @@ static const struct vc4_pv_data bcm2711_pv1_data = {
>
> static const struct vc4_pv_data bcm2711_pv2_data = {
> .base = {
> + .debugfs_name = "crtc2_regs",
> .hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
> .hvs_output = 4,
> },
> - .debugfs_name = "crtc2_regs",
> .fifo_depth = 256,
> .pixels_per_clock = 2,
> .encoder_types = {
> @@ -1061,10 +1061,10 @@ static const struct vc4_pv_data bcm2711_pv2_data = {
>
> static const struct vc4_pv_data bcm2711_pv3_data = {
> .base = {
> + .debugfs_name = "crtc3_regs",
> .hvs_available_channels = BIT(1),
> .hvs_output = 1,
> },
> - .debugfs_name = "crtc3_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 1,
> .encoder_types = {
> @@ -1074,10 +1074,10 @@ static const struct vc4_pv_data bcm2711_pv3_data = {
>
> static const struct vc4_pv_data bcm2711_pv4_data = {
> .base = {
> + .debugfs_name = "crtc4_regs",
> .hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
> .hvs_output = 5,
> },
> - .debugfs_name = "crtc4_regs",
> .fifo_depth = 64,
> .pixels_per_clock = 2,
> .encoder_types = {
> @@ -1214,7 +1214,7 @@ static int vc4_crtc_bind(struct device *dev, struct
> device *master, void *data)
>
> platform_set_drvdata(pdev, vc4_crtc);
>
> - vc4_debugfs_add_regset32(drm, pv_data->debugfs_name,
> + vc4_debugfs_add_regset32(drm, pv_data->base.debugfs_name,
> &vc4_crtc->regset);
>
> return 0;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 5125ca1a8158..9a53ace85d95 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -457,
[PATCH 17/64] drm/vc4: crtc: Move debugfs_name to crtc_data
All the CRTCs, including the TXP, have a debugfs file and name so we can
consolidate it into vc4_crtc_data.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/vc4/vc4_crtc.c | 18 +-
drivers/gpu/drm/vc4/vc4_drv.h | 4 ++--
drivers/gpu/drm/vc4/vc4_txp.c | 1 +
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 7163f924b48b..1f7e987e68aa 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -978,10 +978,10 @@ static const struct drm_crtc_helper_funcs
vc4_crtc_helper_funcs = {
static const struct vc4_pv_data bcm2835_pv0_data = {
.base = {
+ .debugfs_name = "crtc0_regs",
.hvs_available_channels = BIT(0),
.hvs_output = 0,
},
- .debugfs_name = "crtc0_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -992,10 +992,10 @@ static const struct vc4_pv_data bcm2835_pv0_data = {
static const struct vc4_pv_data bcm2835_pv1_data = {
.base = {
+ .debugfs_name = "crtc1_regs",
.hvs_available_channels = BIT(2),
.hvs_output = 2,
},
- .debugfs_name = "crtc1_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1006,10 +1006,10 @@ static const struct vc4_pv_data bcm2835_pv1_data = {
static const struct vc4_pv_data bcm2835_pv2_data = {
.base = {
+ .debugfs_name = "crtc2_regs",
.hvs_available_channels = BIT(1),
.hvs_output = 1,
},
- .debugfs_name = "crtc2_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1020,10 +1020,10 @@ static const struct vc4_pv_data bcm2835_pv2_data = {
static const struct vc4_pv_data bcm2711_pv0_data = {
.base = {
+ .debugfs_name = "crtc0_regs",
.hvs_available_channels = BIT(0),
.hvs_output = 0,
},
- .debugfs_name = "crtc0_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1034,10 +1034,10 @@ static const struct vc4_pv_data bcm2711_pv0_data = {
static const struct vc4_pv_data bcm2711_pv1_data = {
.base = {
+ .debugfs_name = "crtc1_regs",
.hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
.hvs_output = 3,
},
- .debugfs_name = "crtc1_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1048,10 +1048,10 @@ static const struct vc4_pv_data bcm2711_pv1_data = {
static const struct vc4_pv_data bcm2711_pv2_data = {
.base = {
+ .debugfs_name = "crtc2_regs",
.hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
.hvs_output = 4,
},
- .debugfs_name = "crtc2_regs",
.fifo_depth = 256,
.pixels_per_clock = 2,
.encoder_types = {
@@ -1061,10 +1061,10 @@ static const struct vc4_pv_data bcm2711_pv2_data = {
static const struct vc4_pv_data bcm2711_pv3_data = {
.base = {
+ .debugfs_name = "crtc3_regs",
.hvs_available_channels = BIT(1),
.hvs_output = 1,
},
- .debugfs_name = "crtc3_regs",
.fifo_depth = 64,
.pixels_per_clock = 1,
.encoder_types = {
@@ -1074,10 +1074,10 @@ static const struct vc4_pv_data bcm2711_pv3_data = {
static const struct vc4_pv_data bcm2711_pv4_data = {
.base = {
+ .debugfs_name = "crtc4_regs",
.hvs_available_channels = BIT(0) | BIT(1) | BIT(2),
.hvs_output = 5,
},
- .debugfs_name = "crtc4_regs",
.fifo_depth = 64,
.pixels_per_clock = 2,
.encoder_types = {
@@ -1214,7 +1214,7 @@ static int vc4_crtc_bind(struct device *dev, struct
device *master, void *data)
platform_set_drvdata(pdev, vc4_crtc);
- vc4_debugfs_add_regset32(drm, pv_data->debugfs_name,
+ vc4_debugfs_add_regset32(drm, pv_data->base.debugfs_name,
&vc4_crtc->regset);
return 0;
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 5125ca1a8158..9a53ace85d95 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -457,6 +457,8 @@ to_vc4_encoder(struct drm_encoder *encoder)
}
struct vc4_crtc_data {
+ const char *debugfs_name;
+
/* Bitmask of channels (FIFOs) of the HVS that the output can source
from */
unsigned int hvs_available_channels;
@@ -474,8 +476,6 @@ struct vc4_pv_data {
u8 pixels_per_clock;
enum vc4_encoder_type encoder_types[4];
- const char *debugfs_name;
-
};
struct vc4_crtc {
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 82beb8c159f2..e983ff7c5e13 100644
--- a/
