| Message ID | 20220610092924.754942-18-maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Fix hotplug for vc4 | expand |
On Fri, 10 Jun 2022 at 10:30, Maxime Ripard <maxime@cerno.tech> 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 <maxime@cerno.tech> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> 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,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/drivers/gpu/drm/vc4/vc4_txp.c > +++ b/drivers/gpu/drm/vc4/vc4_txp.c > @@ -460,6 +460,7 @@ static irqreturn_t vc4_txp_interrupt(int irq, void *data) > } > > static const struct vc4_crtc_data vc4_txp_crtc_data = { > + .debugfs_name = "txp_regs", > .hvs_available_channels = BIT(2), > .hvs_output = 2, > }; > -- > 2.36.1 >
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 <maxime@cerno.tech> 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 <maxime@cerno.tech> > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > 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-maxime@cerno.tech/ Maxime
On Thu, 16 Jun 2022 at 10:41, Maxime Ripard <maxime@cerno.tech> 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 <maxime@cerno.tech> 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 <maxime@cerno.tech> > > > > Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > > 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-maxime@cerno.tech/ Indeed, and that cleans it all up. Perfect. Dave
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/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -460,6 +460,7 @@ static irqreturn_t vc4_txp_interrupt(int irq, void *data) } static const struct vc4_crtc_data vc4_txp_crtc_data = { + .debugfs_name = "txp_regs", .hvs_available_channels = BIT(2), .hvs_output = 2, };
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 <maxime@cerno.tech> --- 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(-)