| Message ID | 4cd617827cc28875ef36f3632122a83cff2ea4a7.1587742492.git-series.maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Support BCM2711 Display Pipeline | expand |
Hi Maxime, Am 24.04.20 um 17:35 schrieb Maxime Ripard: > The HDMI driver was registering a single debugfs file so far with the name > hdmi_regs. > > Obviously, this is not going to work anymore when will have multiple HDMI > controllers since we will end up trying to register two files with the same > name. > > Let's use the ID to avoid that name conflict. even with this patch there is a name conflict in debugfs using Linux 5.7-rc1. Dave Stevenson addressed this by using different card names [1]. Since this patch won't apply anymore here is my suggestion: diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 29287ab..7209397 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1181,9 +1181,14 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) card->dai_link = dai_link; card->num_links = 1; - card->name = "vc4-hdmi"; card->dev = dev; + if (vc4_hdmi->variant->encoder_type == VC4_ENCODER_TYPE_HDMI1) { + card->name = "vc4-hdmi1"; + } else { + card->name = "vc4-hdmi"; + } + /* * Be careful, snd_soc_register_card() calls dev_set_drvdata() and * stores a pointer to the snd card object in dev->driver_data. This
Hi Stefan, On Sat, Apr 25, 2020 at 11:26:31PM +0200, Stefan Wahren wrote: > Am 24.04.20 um 17:35 schrieb Maxime Ripard: > > The HDMI driver was registering a single debugfs file so far with the name > > hdmi_regs. > > > > Obviously, this is not going to work anymore when will have multiple HDMI > > controllers since we will end up trying to register two files with the same > > name. > > > > Let's use the ID to avoid that name conflict. > > even with this patch there is a name conflict in debugfs using Linux > 5.7-rc1. Dave Stevenson addressed this by using different card names > [1]. Since this patch won't apply anymore here is my suggestion: > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > index 29287ab..7209397 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > @@ -1181,9 +1181,14 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi > *vc4_hdmi) > > card->dai_link = dai_link; > card->num_links = 1; > - card->name = "vc4-hdmi"; > card->dev = dev; > > + if (vc4_hdmi->variant->encoder_type == VC4_ENCODER_TYPE_HDMI1) { > + card->name = "vc4-hdmi1"; > + } else { > + card->name = "vc4-hdmi"; > + } > + Thinking about this some more, we don't really need VC4_ENCODER_TYPE_HDMI0 and HDMI1, and it can all work with the same encoder type for both, so I'll drop them. To address this issue though, we can add a card name string to the variant, like I did for debugfs. Is that alright for you? Maxime
Hi Stefan and Maxime On Tue, 28 Apr 2020 at 16:57, Maxime Ripard <maxime@cerno.tech> wrote: > > Hi Stefan, > > On Sat, Apr 25, 2020 at 11:26:31PM +0200, Stefan Wahren wrote: > > Am 24.04.20 um 17:35 schrieb Maxime Ripard: > > > The HDMI driver was registering a single debugfs file so far with the name > > > hdmi_regs. > > > > > > Obviously, this is not going to work anymore when will have multiple HDMI > > > controllers since we will end up trying to register two files with the same > > > name. > > > > > > Let's use the ID to avoid that name conflict. > > > > even with this patch there is a name conflict in debugfs using Linux > > 5.7-rc1. Dave Stevenson addressed this by using different card names > > [1]. Since this patch won't apply anymore here is my suggestion: > > > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > > index 29287ab..7209397 100644 > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > > @@ -1181,9 +1181,14 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi > > *vc4_hdmi) > > > > card->dai_link = dai_link; > > card->num_links = 1; > > - card->name = "vc4-hdmi"; > > card->dev = dev; > > > > + if (vc4_hdmi->variant->encoder_type == VC4_ENCODER_TYPE_HDMI1) { > > + card->name = "vc4-hdmi1"; > > + } else { > > + card->name = "vc4-hdmi"; > > + } > > + > > Thinking about this some more, we don't really need VC4_ENCODER_TYPE_HDMI0 and > HDMI1, and it can all work with the same encoder type for both, so I'll drop > them. > > To address this issue though, we can add a card name string to the variant, like > I did for debugfs. Is that alright for you? My patch doesn't fix everything with the audio debugfs entries anyway. I'm working against 5.4 on our downstream tree, and even with my patch I get [ 7.459508] debugfs: Directory 'fef00700.hdmi' with parent 'vc4-hdmi' already present! [ 7.511017] debugfs: Directory 'fef05700.hdmi' with parent 'vc4-hdmi1' already present! I seem to recall I reduced the number of complaints over 'vc4-hdmi', but internal to sound/soc-core the node is still registered twice. There was discussion about this a few months back. https://lore.kernel.org/lkml/1jblpvraho.fsf@starbuckisacylon.baylibre.com/ seemed to conclude that it wasn't totally trivial to solve. Regards, Dave
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index aaf96420d0ec..aae5b10a2d11 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1377,7 +1377,9 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_destroy_encoder; - vc4_debugfs_add_file(drm, "hdmi_regs", vc4_hdmi_debugfs_regs, vc4_hdmi); + vc4_debugfs_add_file(drm, variant->debugfs_name, + vc4_hdmi_debugfs_regs, + vc4_hdmi); return 0; @@ -1439,6 +1441,7 @@ static int vc4_hdmi_dev_remove(struct platform_device *pdev) static const struct vc4_hdmi_variant bcm2835_variant = { .encoder_type = VC4_ENCODER_TYPE_HDMI0, + .debugfs_name = "hdmi_regs", .registers = vc4_hdmi_fields, .num_registers = ARRAY_SIZE(vc4_hdmi_fields), diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 4240c5ea7fde..22100820c81b 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -30,6 +30,9 @@ struct vc4_hdmi_variant { /* Encoder Type for that controller */ enum vc4_encoder_type encoder_type; + /* Filename to expose the registers in debugfs */ + const char *debugfs_name; + /* List of the registers available on that variant */ const struct vc4_hdmi_register *registers;
The HDMI driver was registering a single debugfs file so far with the name hdmi_regs. Obviously, this is not going to work anymore when will have multiple HDMI controllers since we will end up trying to register two files with the same name. Let's use the ID to avoid that name conflict. Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++++- drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)