From f1bc386b319e93e56453ae27e9e83817bb1f6f95 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Wed, 29 Jun 2022 14:34:36 +0200 Subject: drm/vc4: hdmi: Depends on CONFIG_PM We already depend on runtime PM to get the power domains and clocks for most of the devices supported by the vc4 driver, so let's just select it to make sure it's there. Link: https://lore.kernel.org/r/20220629123510.1915022-38-maxime@cerno.tech Acked-by: Thomas Zimmermann Tested-by: Stefan Wahren Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index e078f4ce0b4b..4a2e24421089 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2854,7 +2854,7 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) return 0; } -static int __maybe_unused vc4_hdmi_runtime_suspend(struct device *dev) +static int vc4_hdmi_runtime_suspend(struct device *dev) { struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev); -- cgit v1.2.3 From 53565c28e6af2cef6bbf438c34250135e3564459 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Wed, 29 Jun 2022 14:34:37 +0200 Subject: drm/vc4: hdmi: Rework power up The current code tries to handle the case where CONFIG_PM isn't selected by first calling our runtime_resume implementation and then properly report the power state to the runtime_pm core. This allows to have a functionning device even if pm_runtime_get_* functions are nops. However, the device power state if CONFIG_PM is enabled is RPM_SUSPENDED, and thus our vc4_hdmi_write() and vc4_hdmi_read() calls in the runtime_pm hooks will now report a warning since the device might not be properly powered. Even more so, we need CONFIG_PM enabled since the previous RaspberryPi have a power domain that needs to be powered up for the HDMI controller to be usable. The previous patch has created a dependency on CONFIG_PM, now we can just assume it's there and only call pm_runtime_resume_and_get() to make sure our device is powered in bind. Link: https://lore.kernel.org/r/20220629123510.1915022-39-maxime@cerno.tech Acked-by: Thomas Zimmermann Tested-by: Stefan Wahren Signed-off-by: Maxime Ripard --- drivers/gpu/drm/vc4/vc4_hdmi.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4a2e24421089..e77bdc37b16f 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2971,17 +2971,15 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) vc4_hdmi->disable_4kp60 = true; } + pm_runtime_enable(dev); + /* - * We need to have the device powered up at this point to call - * our reset hook and for the CEC init. + * We need to have the device powered up at this point to call + * our reset hook and for the CEC init. */ - ret = vc4_hdmi_runtime_resume(dev); + ret = pm_runtime_resume_and_get(dev); if (ret) - goto err_put_ddc; - - pm_runtime_get_noresume(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); + goto err_disable_runtime_pm; if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") || of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) && @@ -3027,6 +3025,7 @@ err_destroy_conn: err_destroy_encoder: drm_encoder_cleanup(encoder); pm_runtime_put_sync(dev); +err_disable_runtime_pm: pm_runtime_disable(dev); err_put_ddc: put_device(&vc4_hdmi->ddc->dev); -- cgit v1.2.3 From b4f2c70c1a7a0a7708c9755a9946452ee23adeb6 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:07 +0200 Subject: drm/vc4: hdmi: Switch to drmm_kzalloc Our internal structure that stores the DRM entities structure is allocated through a device-managed kzalloc. This means that this will eventually be freed whenever the device is removed. In our case, the most likely source of removal is that the main device is going to be unbound, and component_unbind_all() is being run. However, it occurs while the DRM device is still registered, which will create dangling pointers, eventually resulting in use-after-free. Switch to a DRM-managed allocation to keep our structure until the DRM driver doesn't need it anymore. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-38-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 73fb2f91c3e4..fba549edcfc5 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2907,9 +2907,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) struct device_node *ddc_node; int ret; - vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL); + vc4_hdmi = drmm_kzalloc(drm, sizeof(*vc4_hdmi), GFP_KERNEL); if (!vc4_hdmi) return -ENOMEM; + mutex_init(&vc4_hdmi->mutex); spin_lock_init(&vc4_hdmi->hw_lock); INIT_DELAYED_WORK(&vc4_hdmi->scrambling_work, vc4_hdmi_scrambling_wq); -- cgit v1.2.3 From 7a951e3a26920ae424f563f63925bf478b37d00b Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:08 +0200 Subject: drm/vc4: hdmi: Remove call to drm_connector_unregister() drm_connector_unregister() is only to be used for connectors that have been registered through drm_connector_register() after drm_dev_register() has been called. This is our case here so let's remove the call. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-39-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index fba549edcfc5..05f769474903 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -267,12 +267,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) return connector_status_disconnected; } -static void vc4_hdmi_connector_destroy(struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) { struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); @@ -380,7 +374,7 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { .detect = vc4_hdmi_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = vc4_hdmi_connector_destroy, + .destroy = drm_connector_cleanup, .reset = vc4_hdmi_connector_reset, .atomic_duplicate_state = vc4_hdmi_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, @@ -3022,7 +3016,7 @@ err_free_cec: err_free_hotplug: vc4_hdmi_hotplug_exit(vc4_hdmi); err_destroy_conn: - vc4_hdmi_connector_destroy(&vc4_hdmi->connector); + drm_connector_cleanup(&vc4_hdmi->connector); err_destroy_encoder: drm_encoder_cleanup(encoder); pm_runtime_put_sync(dev); @@ -3066,7 +3060,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, vc4_hdmi_audio_exit(vc4_hdmi); vc4_hdmi_cec_exit(vc4_hdmi); vc4_hdmi_hotplug_exit(vc4_hdmi); - vc4_hdmi_connector_destroy(&vc4_hdmi->connector); + drm_connector_cleanup(&vc4_hdmi->connector); drm_encoder_cleanup(&vc4_hdmi->encoder.base); pm_runtime_disable(dev); -- cgit v1.2.3 From 56924791cd5de99374a027cf00ce9b6e55d74203 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:09 +0200 Subject: drm/vc4: hdmi: Switch to DRM-managed encoder initialization The current code will call drm_encoder_cleanup() when the device is unbound. However, by then, there might still be some references held to that encoder, including by the userspace that might still have the DRM device open. Let's switch to a DRM-managed initialization to clean up after ourselves only once the DRM device has been last closed. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-40-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 05f769474903..6c44faea4af1 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2984,12 +2984,18 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); } - drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS); + ret = drmm_encoder_init(drm, encoder, + NULL, + DRM_MODE_ENCODER_TMDS, + NULL); + if (ret) + goto err_put_runtime_pm; + drm_encoder_helper_add(encoder, &vc4_hdmi_encoder_helper_funcs); ret = vc4_hdmi_connector_init(drm, vc4_hdmi); if (ret) - goto err_destroy_encoder; + goto err_put_runtime_pm; ret = vc4_hdmi_hotplug_init(vc4_hdmi); if (ret) @@ -3017,8 +3023,7 @@ err_free_hotplug: vc4_hdmi_hotplug_exit(vc4_hdmi); err_destroy_conn: drm_connector_cleanup(&vc4_hdmi->connector); -err_destroy_encoder: - drm_encoder_cleanup(encoder); +err_put_runtime_pm: pm_runtime_put_sync(dev); err_disable_runtime_pm: pm_runtime_disable(dev); @@ -3061,7 +3066,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, vc4_hdmi_cec_exit(vc4_hdmi); vc4_hdmi_hotplug_exit(vc4_hdmi); drm_connector_cleanup(&vc4_hdmi->connector); - drm_encoder_cleanup(&vc4_hdmi->encoder.base); pm_runtime_disable(dev); -- cgit v1.2.3 From 19d094c3a17d228381d4f1a25158a7b91b0e294f Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:10 +0200 Subject: drm/vc4: hdmi: Switch to DRM-managed connector initialization The current code will call drm_connector_unregister() and drm_connector_cleanup() when the device is unbound. However, by then, there might still be some references held to that connector, including by the userspace that might still have the DRM device open. Let's switch to a DRM-managed initialization to clean up after ourselves only once the DRM device has been last closed. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-41-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 6c44faea4af1..283fea78f3cf 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -374,7 +374,6 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { .detect = vc4_hdmi_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = drm_connector_cleanup, .reset = vc4_hdmi_connector_reset, .atomic_duplicate_state = vc4_hdmi_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, @@ -392,10 +391,13 @@ static int vc4_hdmi_connector_init(struct drm_device *dev, struct drm_encoder *encoder = &vc4_hdmi->encoder.base; int ret; - drm_connector_init_with_ddc(dev, connector, - &vc4_hdmi_connector_funcs, - DRM_MODE_CONNECTOR_HDMIA, - vc4_hdmi->ddc); + ret = drmm_connector_init(dev, connector, + &vc4_hdmi_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA, + vc4_hdmi->ddc); + if (ret) + return ret; + drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs); /* @@ -2999,7 +3001,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) ret = vc4_hdmi_hotplug_init(vc4_hdmi); if (ret) - goto err_destroy_conn; + goto err_put_runtime_pm; ret = vc4_hdmi_cec_init(vc4_hdmi); if (ret) @@ -3021,8 +3023,6 @@ err_free_cec: vc4_hdmi_cec_exit(vc4_hdmi); err_free_hotplug: vc4_hdmi_hotplug_exit(vc4_hdmi); -err_destroy_conn: - drm_connector_cleanup(&vc4_hdmi->connector); err_put_runtime_pm: pm_runtime_put_sync(dev); err_disable_runtime_pm: @@ -3065,7 +3065,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, vc4_hdmi_audio_exit(vc4_hdmi); vc4_hdmi_cec_exit(vc4_hdmi); vc4_hdmi_hotplug_exit(vc4_hdmi); - drm_connector_cleanup(&vc4_hdmi->connector); pm_runtime_disable(dev); -- cgit v1.2.3 From 59f9d46af43c884b92f690934263196387437c7e Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:11 +0200 Subject: drm/vc4: hdmi: Switch to device-managed ALSA initialization The current code to unregister our ALSA device needs to be undone manually when we remove the HDMI driver. Since ALSA doesn't seem to support any mechanism to defer freeing something until the last user of the ALSA device is gone, we can either use a device-managed or a DRM-managed action. The consistent way would be to use a DRM-managed one, just like pretty much any framework-facing structure should be doing. However, ALSA does a lot of allocation and registration using device-managed calls. Thus, if we're going that way, by the time the DRM-managed action would run all of those allocation would have been freed and we would end up with a use-after-free. Thus, let's do a device-managed action. It's been tested with KASAN enabled and doesn't seem to trigger any issue, so it's as good as anything. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-42-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 43 +++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 283fea78f3cf..bf7d56bbcba0 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2056,6 +2056,14 @@ static struct hdmi_codec_pdata vc4_hdmi_codec_pdata = { .i2s = 1, }; +static void vc4_hdmi_audio_codec_release(void *ptr) +{ + struct vc4_hdmi *vc4_hdmi = ptr; + + platform_device_unregister(vc4_hdmi->audio.codec_pdev); + vc4_hdmi->audio.codec_pdev = NULL; +} + static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) { const struct vc4_hdmi_register *mai_data = @@ -2097,6 +2105,30 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) vc4_hdmi->audio.dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; vc4_hdmi->audio.dma_data.maxburst = 2; + /* + * NOTE: Strictly speaking, we should probably use a DRM-managed + * registration there to avoid removing all the audio components + * by the time the driver doesn't have any user anymore. + * + * However, the ASoC core uses a number of devm_kzalloc calls + * when registering, even when using non-device-managed + * functions (such as in snd_soc_register_component()). + * + * If we call snd_soc_unregister_component() in a DRM-managed + * action, the device-managed actions have already been executed + * and thus we would access memory that has been freed. + * + * Using device-managed hooks here probably leaves us open to a + * bunch of issues if userspace still has a handle on the ALSA + * device when the device is removed. However, this is mitigated + * by the use of drm_dev_enter()/drm_dev_exit() in the audio + * path to prevent the access to the device resources if it + * isn't there anymore. + * + * Then, the vc4_hdmi structure is DRM-managed and thus only + * freed whenever the last user has closed the DRM device file. + * It should thus outlive ALSA in most situations. + */ ret = devm_snd_dmaengine_pcm_register(dev, &pcm_conf, 0); if (ret) { dev_err(dev, "Could not register PCM component: %d\n", ret); @@ -2120,6 +2152,10 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) } vc4_hdmi->audio.codec_pdev = codec_pdev; + ret = devm_add_action_or_reset(dev, vc4_hdmi_audio_codec_release, vc4_hdmi); + if (ret) + return ret; + dai_link->cpus = &vc4_hdmi->audio.cpu; dai_link->codecs = &vc4_hdmi->audio.codec; dai_link->platforms = &vc4_hdmi->audio.platform; @@ -2158,12 +2194,6 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) } -static void vc4_hdmi_audio_exit(struct vc4_hdmi *vc4_hdmi) -{ - platform_device_unregister(vc4_hdmi->audio.codec_pdev); - vc4_hdmi->audio.codec_pdev = NULL; -} - static irqreturn_t vc4_hdmi_hpd_irq_thread(int irq, void *priv) { struct vc4_hdmi *vc4_hdmi = priv; @@ -3062,7 +3092,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, kfree(vc4_hdmi->hdmi_regset.regs); kfree(vc4_hdmi->hd_regset.regs); - vc4_hdmi_audio_exit(vc4_hdmi); vc4_hdmi_cec_exit(vc4_hdmi); vc4_hdmi_hotplug_exit(vc4_hdmi); -- cgit v1.2.3 From 015653f7af7cb7fa56e9a88af2a15c814a39b6b4 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:12 +0200 Subject: drm/vc4: hdmi: Switch to device-managed CEC initialization The current code to unregister our CEC device needs to be undone manually when we remove the HDMI driver. Since the CEC framework will allocate its main structure, and will defer its deallocation to when the last user will have closed it, we don't really need to take any particular measure to prevent any use-after-free and can thus use any managed action. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-43-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 94 ++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 44 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index bf7d56bbcba0..83ca1c02dde2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2576,6 +2576,14 @@ static const struct cec_adap_ops vc4_hdmi_cec_adap_ops = { .adap_transmit = vc4_hdmi_cec_adap_transmit, }; +static void vc4_hdmi_cec_release(void *ptr) +{ + struct vc4_hdmi *vc4_hdmi = ptr; + + cec_unregister_adapter(vc4_hdmi->cec_adap); + vc4_hdmi->cec_adap = NULL; +} + static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) { struct cec_connector_info conn_info; @@ -2600,70 +2608,71 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info); if (vc4_hdmi->variant->external_irq_controller) { - ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-rx"), - vc4_cec_irq_handler_rx_bare, - vc4_cec_irq_handler_rx_thread, 0, - "vc4 hdmi cec rx", vc4_hdmi); + ret = devm_request_threaded_irq(dev, platform_get_irq_byname(pdev, "cec-rx"), + vc4_cec_irq_handler_rx_bare, + vc4_cec_irq_handler_rx_thread, 0, + "vc4 hdmi cec rx", vc4_hdmi); if (ret) goto err_delete_cec_adap; - ret = request_threaded_irq(platform_get_irq_byname(pdev, "cec-tx"), - vc4_cec_irq_handler_tx_bare, - vc4_cec_irq_handler_tx_thread, 0, - "vc4 hdmi cec tx", vc4_hdmi); + ret = devm_request_threaded_irq(dev, platform_get_irq_byname(pdev, "cec-tx"), + vc4_cec_irq_handler_tx_bare, + vc4_cec_irq_handler_tx_thread, 0, + "vc4 hdmi cec tx", vc4_hdmi); if (ret) - goto err_remove_cec_rx_handler; + goto err_delete_cec_adap; } else { - ret = request_threaded_irq(platform_get_irq(pdev, 0), - vc4_cec_irq_handler, - vc4_cec_irq_handler_thread, 0, - "vc4 hdmi cec", vc4_hdmi); + ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0), + vc4_cec_irq_handler, + vc4_cec_irq_handler_thread, 0, + "vc4 hdmi cec", vc4_hdmi); if (ret) goto err_delete_cec_adap; } ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev); if (ret < 0) - goto err_remove_handlers; - - return 0; + goto err_delete_cec_adap; -err_remove_handlers: - if (vc4_hdmi->variant->external_irq_controller) - free_irq(platform_get_irq_byname(pdev, "cec-tx"), vc4_hdmi); - else - free_irq(platform_get_irq(pdev, 0), vc4_hdmi); + /* + * NOTE: Strictly speaking, we should probably use a DRM-managed + * registration there to avoid removing the CEC adapter by the + * time the DRM driver doesn't have any user anymore. + * + * However, the CEC framework already cleans up the CEC adapter + * only when the last user has closed its file descriptor, so we + * don't need to handle it in DRM. + * + * By the time the device-managed hook is executed, we will give + * up our reference to the CEC adapter and therefore don't + * really care when it's actually freed. + * + * There's still a problematic sequence: if we unregister our + * CEC adapter, but the userspace keeps a handle on the CEC + * adapter but not the DRM device for some reason. In such a + * case, our vc4_hdmi structure will be freed, but the + * cec_adapter structure will have a dangling pointer to what + * used to be our HDMI controller. If we get a CEC call at that + * moment, we could end up with a use-after-free. Fortunately, + * the CEC framework already handles this too, by calling + * cec_is_registered() in cec_ioctl() and cec_poll(). + */ + ret = devm_add_action_or_reset(dev, vc4_hdmi_cec_release, vc4_hdmi); + if (ret) + return ret; -err_remove_cec_rx_handler: - if (vc4_hdmi->variant->external_irq_controller) - free_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_hdmi); + return 0; err_delete_cec_adap: cec_delete_adapter(vc4_hdmi->cec_adap); return ret; } - -static void vc4_hdmi_cec_exit(struct vc4_hdmi *vc4_hdmi) -{ - struct platform_device *pdev = vc4_hdmi->pdev; - - if (vc4_hdmi->variant->external_irq_controller) { - free_irq(platform_get_irq_byname(pdev, "cec-rx"), vc4_hdmi); - free_irq(platform_get_irq_byname(pdev, "cec-tx"), vc4_hdmi); - } else { - free_irq(platform_get_irq(pdev, 0), vc4_hdmi); - } - - cec_unregister_adapter(vc4_hdmi->cec_adap); -} #else static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) { return 0; } - -static void vc4_hdmi_cec_exit(struct vc4_hdmi *vc4_hdmi) {}; #endif static int vc4_hdmi_build_regset(struct vc4_hdmi *vc4_hdmi, @@ -3039,7 +3048,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) ret = vc4_hdmi_audio_init(vc4_hdmi); if (ret) - goto err_free_cec; + goto err_free_hotplug; vc4_debugfs_add_file(drm, variant->debugfs_name, vc4_hdmi_debugfs_regs, @@ -3049,8 +3058,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) return 0; -err_free_cec: - vc4_hdmi_cec_exit(vc4_hdmi); err_free_hotplug: vc4_hdmi_hotplug_exit(vc4_hdmi); err_put_runtime_pm: @@ -3092,7 +3099,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, kfree(vc4_hdmi->hdmi_regset.regs); kfree(vc4_hdmi->hd_regset.regs); - vc4_hdmi_cec_exit(vc4_hdmi); vc4_hdmi_hotplug_exit(vc4_hdmi); pm_runtime_disable(dev); -- cgit v1.2.3 From d3a84242f897506509d1bb29b8e64166958fb991 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:13 +0200 Subject: drm/vc4: hdmi: Use a device-managed action for DDC The reference to the DDC controller device needs to be put back when we're done with it. Let's use a device-managed action to simplify the driver. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-44-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 83ca1c02dde2..274f17880455 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2932,6 +2932,13 @@ static int vc4_hdmi_runtime_resume(struct device *dev) return 0; } +static void vc4_hdmi_put_ddc_device(void *ptr) +{ + struct vc4_hdmi *vc4_hdmi = ptr; + + put_device(&vc4_hdmi->ddc->dev); +} + static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) { const struct vc4_hdmi_variant *variant = of_device_get_match_data(dev); @@ -2987,13 +2994,16 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) return -EPROBE_DEFER; } + ret = devm_add_action_or_reset(dev, vc4_hdmi_put_ddc_device, vc4_hdmi); + if (ret) + return ret; + /* Only use the GPIO HPD pin if present in the DT, otherwise * we'll use the HDMI core's register. */ vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN); if (IS_ERR(vc4_hdmi->hpd_gpio)) { - ret = PTR_ERR(vc4_hdmi->hpd_gpio); - goto err_put_ddc; + return PTR_ERR(vc4_hdmi->hpd_gpio); } vc4_hdmi->disable_wifi_frequencies = @@ -3064,8 +3074,6 @@ err_put_runtime_pm: pm_runtime_put_sync(dev); err_disable_runtime_pm: pm_runtime_disable(dev); -err_put_ddc: - put_device(&vc4_hdmi->ddc->dev); return ret; } @@ -3102,8 +3110,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, vc4_hdmi_hotplug_exit(vc4_hdmi); pm_runtime_disable(dev); - - put_device(&vc4_hdmi->ddc->dev); } static const struct component_ops vc4_hdmi_ops = { -- cgit v1.2.3 From a3dbb1c016576838a1591d2341641cb4067840e8 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:14 +0200 Subject: drm/vc4: hdmi: Switch to DRM-managed kfree to build regsets The current code to build the registers set later exposed in debugfs for the HDMI controller relies on traditional allocations, that are later free'd as part of the driver unbind hook. Since krealloc doesn't have a DRM-managed equivalent, let's add an action to free the buffer later on. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-45-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 46 ++++++++++++++++++++++++++---------------- drivers/gpu/drm/vc4/vc4_hdmi.h | 3 ++- 2 files changed, 31 insertions(+), 18 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 274f17880455..c93100fe4b42 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2675,7 +2675,15 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi) } #endif -static int vc4_hdmi_build_regset(struct vc4_hdmi *vc4_hdmi, +static void vc4_hdmi_free_regset(struct drm_device *drm, void *ptr) +{ + struct debugfs_reg32 *regs = ptr; + + kfree(regs); +} + +static int vc4_hdmi_build_regset(struct drm_device *drm, + struct vc4_hdmi *vc4_hdmi, struct debugfs_regset32 *regset, enum vc4_hdmi_regs reg) { @@ -2683,6 +2691,7 @@ static int vc4_hdmi_build_regset(struct vc4_hdmi *vc4_hdmi, struct debugfs_reg32 *regs, *new_regs; unsigned int count = 0; unsigned int i; + int ret; regs = kcalloc(variant->num_registers, sizeof(*regs), GFP_KERNEL); @@ -2708,10 +2717,15 @@ static int vc4_hdmi_build_regset(struct vc4_hdmi *vc4_hdmi, regset->regs = new_regs; regset->nregs = count; + ret = drmm_add_action_or_reset(drm, vc4_hdmi_free_regset, new_regs); + if (ret) + return ret; + return 0; } -static int vc4_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) +static int vc4_hdmi_init_resources(struct drm_device *drm, + struct vc4_hdmi *vc4_hdmi) { struct platform_device *pdev = vc4_hdmi->pdev; struct device *dev = &pdev->dev; @@ -2725,11 +2739,11 @@ static int vc4_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) if (IS_ERR(vc4_hdmi->hd_regs)) return PTR_ERR(vc4_hdmi->hd_regs); - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI); if (ret) return ret; @@ -2752,7 +2766,8 @@ static int vc4_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) return 0; } -static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) +static int vc5_hdmi_init_resources(struct drm_device *drm, + struct vc4_hdmi *vc4_hdmi) { struct platform_device *pdev = vc4_hdmi->pdev; struct device *dev = &pdev->dev; @@ -2854,35 +2869,35 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) return PTR_ERR(vc4_hdmi->reset); } - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hdmi_regset, VC4_HDMI); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->hd_regset, VC4_HD); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->cec_regset, VC5_CEC); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->cec_regset, VC5_CEC); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->csc_regset, VC5_CSC); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->csc_regset, VC5_CSC); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->dvp_regset, VC5_DVP); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->dvp_regset, VC5_DVP); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->phy_regset, VC5_PHY); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->phy_regset, VC5_PHY); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->ram_regset, VC5_RAM); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->ram_regset, VC5_RAM); if (ret) return ret; - ret = vc4_hdmi_build_regset(vc4_hdmi, &vc4_hdmi->rm_regset, VC5_RM); + ret = vc4_hdmi_build_regset(drm, vc4_hdmi, &vc4_hdmi->rm_regset, VC5_RM); if (ret) return ret; @@ -2977,7 +2992,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) if (variant->max_pixel_clock > HDMI_14_MAX_TMDS_CLK) vc4_hdmi->scdc_enabled = true; - ret = variant->init_resources(vc4_hdmi); + ret = variant->init_resources(drm, vc4_hdmi); if (ret) return ret; @@ -3104,9 +3119,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0); vc4_hdmi = dev_get_drvdata(dev); - kfree(vc4_hdmi->hdmi_regset.regs); - kfree(vc4_hdmi->hd_regset.regs); - vc4_hdmi_hotplug_exit(vc4_hdmi); pm_runtime_disable(dev); diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index c3ed2b07df23..99b0bc1297be 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -58,7 +58,8 @@ struct vc4_hdmi_variant { /* Callback to get the resources (memory region, interrupts, * clocks, etc) for that variant. */ - int (*init_resources)(struct vc4_hdmi *vc4_hdmi); + int (*init_resources)(struct drm_device *drm, + struct vc4_hdmi *vc4_hdmi); /* Callback to reset the HDMI block */ void (*reset)(struct vc4_hdmi *vc4_hdmi); -- cgit v1.2.3 From af8a3b125b682c4c582bb25342af4759d9b4a1e5 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:15 +0200 Subject: drm/vc4: hdmi: Use devm to register hotplug interrupts Commit 776efe800fed ("drm/vc4: hdmi: Drop devm interrupt handler for hotplug interrupts") dropped the device-managed interrupt registration because it was creating bugs and races whenever an interrupt was coming in while the device was removed. However, our latest patches to the HDMI controller driver fix this as well, so we can use device-managed interrupt handlers again. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-46-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index c93100fe4b42..6275db463afd 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2216,21 +2216,19 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi *vc4_hdmi) unsigned int hpd_con = platform_get_irq_byname(pdev, "hpd-connected"); unsigned int hpd_rm = platform_get_irq_byname(pdev, "hpd-removed"); - ret = request_threaded_irq(hpd_con, - NULL, - vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT, - "vc4 hdmi hpd connected", vc4_hdmi); + ret = devm_request_threaded_irq(&pdev->dev, hpd_con, + NULL, + vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT, + "vc4 hdmi hpd connected", vc4_hdmi); if (ret) return ret; - ret = request_threaded_irq(hpd_rm, - NULL, - vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT, - "vc4 hdmi hpd disconnected", vc4_hdmi); - if (ret) { - free_irq(hpd_con, vc4_hdmi); + ret = devm_request_threaded_irq(&pdev->dev, hpd_rm, + NULL, + vc4_hdmi_hpd_irq_thread, IRQF_ONESHOT, + "vc4 hdmi hpd disconnected", vc4_hdmi); + if (ret) return ret; - } connector->polled = DRM_CONNECTOR_POLL_HPD; } @@ -2238,16 +2236,6 @@ static int vc4_hdmi_hotplug_init(struct vc4_hdmi *vc4_hdmi) return 0; } -static void vc4_hdmi_hotplug_exit(struct vc4_hdmi *vc4_hdmi) -{ - struct platform_device *pdev = vc4_hdmi->pdev; - - if (vc4_hdmi->variant->external_irq_controller) { - free_irq(platform_get_irq_byname(pdev, "hpd-connected"), vc4_hdmi); - free_irq(platform_get_irq_byname(pdev, "hpd-removed"), vc4_hdmi); - } -} - #ifdef CONFIG_DRM_VC4_HDMI_CEC static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv) { @@ -3069,11 +3057,11 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) ret = vc4_hdmi_cec_init(vc4_hdmi); if (ret) - goto err_free_hotplug; + goto err_put_runtime_pm; ret = vc4_hdmi_audio_init(vc4_hdmi); if (ret) - goto err_free_hotplug; + goto err_put_runtime_pm; vc4_debugfs_add_file(drm, variant->debugfs_name, vc4_hdmi_debugfs_regs, @@ -3083,8 +3071,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) return 0; -err_free_hotplug: - vc4_hdmi_hotplug_exit(vc4_hdmi); err_put_runtime_pm: pm_runtime_put_sync(dev); err_disable_runtime_pm: @@ -3096,8 +3082,6 @@ err_disable_runtime_pm: static void vc4_hdmi_unbind(struct device *dev, struct device *master, void *data) { - struct vc4_hdmi *vc4_hdmi; - /* * ASoC makes it a bit hard to retrieve a pointer to the * vc4_hdmi structure. Registering the card will overwrite our @@ -3117,9 +3101,6 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master, */ BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0); BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0); - vc4_hdmi = dev_get_drvdata(dev); - - vc4_hdmi_hotplug_exit(vc4_hdmi); pm_runtime_disable(dev); } -- cgit v1.2.3 From 0c9d0ddd9cf4b43f8b3faf752c5a1aaa665ed099 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:16 +0200 Subject: drm/vc4: hdmi: Move audio structure offset checks The HDMI driver unbind hook doesn't have any ALSA-related code anymore, so let's move the ALSA sanity checks and comments we have to some other part of the driver dedicated to ALSA. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-47-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 6275db463afd..a130b7d48e46 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -2076,6 +2076,26 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi) int index, len; int ret; + /* + * ASoC makes it a bit hard to retrieve a pointer to the + * vc4_hdmi structure. Registering the card will overwrite our + * device drvdata with a pointer to the snd_soc_card structure, + * which can then be used to retrieve whatever drvdata we want + * to associate. + * + * However, that doesn't fly in the case where we wouldn't + * register an ASoC card (because of an old DT that is missing + * the dmas properties for example), then the card isn't + * registered and the device drvdata wouldn't be set. + * + * We can deal with both cases by making sure a snd_soc_card + * pointer and a vc4_hdmi structure are pointing to the same + * memory address, so we can treat them indistinctly without any + * issue. + */ + BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0); + BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0); + if (!of_find_property(dev->of_node, "dmas", &len) || !len) { dev_warn(dev, "'dmas' DT property is missing or empty, no HDMI audio\n"); @@ -3082,26 +3102,6 @@ err_disable_runtime_pm: static void vc4_hdmi_unbind(struct device *dev, struct device *master, void *data) { - /* - * ASoC makes it a bit hard to retrieve a pointer to the - * vc4_hdmi structure. Registering the card will overwrite our - * device drvdata with a pointer to the snd_soc_card structure, - * which can then be used to retrieve whatever drvdata we want - * to associate. - * - * However, that doesn't fly in the case where we wouldn't - * register an ASoC card (because of an old DT that is missing - * the dmas properties for example), then the card isn't - * registered and the device drvdata wouldn't be set. - * - * We can deal with both cases by making sure a snd_soc_card - * pointer and a vc4_hdmi structure are pointing to the same - * memory address, so we can treat them indistinctly without any - * issue. - */ - BUILD_BUG_ON(offsetof(struct vc4_hdmi_audio, card) != 0); - BUILD_BUG_ON(offsetof(struct vc4_hdmi, audio) != 0); - pm_runtime_disable(dev); } -- cgit v1.2.3 From cd00ed5187bff03d4c2ba143a5993383dd6ed66c Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:17 +0200 Subject: drm/vc4: hdmi: Protect device resources after removal Whenever the device and driver are unbound, the main device and all the subdevices will be removed by calling their unbind() method. However, the DRM device itself will only be freed when the last user will have closed it. It means that there is a time window where the device and its resources aren't there anymore, but the userspace can still call into our driver. Fortunately, the DRM framework provides the drm_dev_enter() and drm_dev_exit() functions to make sure our underlying device is still there for the section protected by those calls. Let's add them to the HDMI driver. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-48-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 308 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 291 insertions(+), 17 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index a130b7d48e46..a826faf8b02d 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -146,7 +147,12 @@ static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) { struct drm_info_node *node = (struct drm_info_node *)m->private; struct vc4_hdmi *vc4_hdmi = node->info_ent->data; + struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_printer p = drm_seq_file_printer(m); + int idx; + + if (!drm_dev_enter(drm, &idx)) + return -ENODEV; drm_print_regset32(&p, &vc4_hdmi->hdmi_regset); drm_print_regset32(&p, &vc4_hdmi->hd_regset); @@ -157,12 +163,23 @@ static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) drm_print_regset32(&p, &vc4_hdmi->ram_regset); drm_print_regset32(&p, &vc4_hdmi->rm_regset); + drm_dev_exit(idx); + return 0; } static void vc4_hdmi_reset(struct vc4_hdmi *vc4_hdmi) { + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; + + /* + * We can be called by our bind callback, when the + * connector->dev pointer might not be initialised yet. + */ + if (drm && !drm_dev_enter(drm, &idx)) + return; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -179,11 +196,23 @@ static void vc4_hdmi_reset(struct vc4_hdmi *vc4_hdmi) HDMI_WRITE(HDMI_SW_RESET_CONTROL, 0); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + if (drm) + drm_dev_exit(idx); } static void vc5_hdmi_reset(struct vc4_hdmi *vc4_hdmi) { + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; + + /* + * We can be called by our bind callback, when the + * connector->dev pointer might not be initialised yet. + */ + if (drm && !drm_dev_enter(drm, &idx)) + return; reset_control_reset(vc4_hdmi->reset); @@ -195,15 +224,31 @@ static void vc5_hdmi_reset(struct vc4_hdmi *vc4_hdmi) HDMI_READ(HDMI_CLOCK_STOP) | VC4_DVP_HT_CLOCK_STOP_PIXEL); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + if (drm) + drm_dev_exit(idx); } #ifdef CONFIG_DRM_VC4_HDMI_CEC static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) { - unsigned long cec_rate = clk_get_rate(vc4_hdmi->cec_clock); + struct drm_device *drm = vc4_hdmi->connector.dev; + unsigned long cec_rate; unsigned long flags; u16 clk_cnt; u32 value; + int idx; + + /* + * This function is called by our runtime_resume implementation + * and thus at bind time, when we haven't registered our + * connector yet and thus don't have a pointer to the DRM + * device. + */ + if (drm && !drm_dev_enter(drm, &idx)) + return; + + cec_rate = clk_get_rate(vc4_hdmi->cec_clock); spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -219,6 +264,9 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) HDMI_WRITE(HDMI_CEC_CNTRL_1, value); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + if (drm) + drm_dev_exit(idx); } #else static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {} @@ -440,25 +488,34 @@ static int vc4_hdmi_stop_packet(struct drm_encoder *encoder, bool poll) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; u32 packet_id = type - 0x80; unsigned long flags; + int ret = 0; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return -ENODEV; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, HDMI_READ(HDMI_RAM_PACKET_CONFIG) & ~BIT(packet_id)); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); - if (!poll) - return 0; + if (poll) { + ret = wait_for(!(HDMI_READ(HDMI_RAM_PACKET_STATUS) & + BIT(packet_id)), 100); + } - return wait_for(!(HDMI_READ(HDMI_RAM_PACKET_STATUS) & - BIT(packet_id)), 100); + drm_dev_exit(idx); + return ret; } static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, union hdmi_infoframe *frame) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; u32 packet_id = frame->any.type - 0x80; const struct vc4_hdmi_register *ram_packet_start = &vc4_hdmi->variant->registers[HDMI_RAM_PACKET_START]; @@ -471,6 +528,10 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, unsigned long flags; ssize_t len, i; int ret; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; WARN_ONCE(!(HDMI_READ(HDMI_RAM_PACKET_CONFIG) & VC4_HDMI_RAM_PACKET_ENABLE), @@ -478,12 +539,12 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, len = hdmi_infoframe_pack(frame, buffer, sizeof(buffer)); if (len < 0) - return; + goto out; ret = vc4_hdmi_stop_packet(encoder, frame->any.type, true); if (ret) { DRM_ERROR("Failed to wait for infoframe to go idle: %d\n", ret); - return; + goto out; } spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -519,6 +580,9 @@ static void vc4_hdmi_write_infoframe(struct drm_encoder *encoder, BIT(packet_id)), 100); if (ret) DRM_ERROR("Failed to wait for infoframe to start: %d\n", ret); + +out: + drm_dev_exit(idx); } static void vc4_hdmi_avi_infoframe_colorspace(struct hdmi_avi_infoframe *frame, @@ -668,8 +732,10 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder, static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; unsigned long flags; + int idx; lockdep_assert_held(&vc4_hdmi->mutex); @@ -681,6 +747,9 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) vc4_hdmi->output_format)) return; + if (!drm_dev_enter(drm, &idx)) + return; + drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true); drm_scdc_set_scrambling(vc4_hdmi->ddc, true); @@ -689,6 +758,8 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) VC5_HDMI_SCRAMBLER_CTL_ENABLE); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + drm_dev_exit(idx); + vc4_hdmi->scdc_enabled = true; queue_delayed_work(system_wq, &vc4_hdmi->scrambling_work, @@ -698,7 +769,9 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; lockdep_assert_held(&vc4_hdmi->mutex); @@ -710,6 +783,9 @@ static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder) if (delayed_work_pending(&vc4_hdmi->scrambling_work)) cancel_delayed_work_sync(&vc4_hdmi->scrambling_work); + if (!drm_dev_enter(drm, &idx)) + return; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) & ~VC5_HDMI_SCRAMBLER_CTL_ENABLE); @@ -717,6 +793,8 @@ static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder) drm_scdc_set_scrambling(vc4_hdmi->ddc, false); drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, false); + + drm_dev_exit(idx); } static void vc4_hdmi_scrambling_wq(struct work_struct *work) @@ -739,12 +817,17 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; mutex_lock(&vc4_hdmi->mutex); vc4_hdmi->packet_ram_enabled = false; + if (!drm_dev_enter(drm, &idx)) + goto out; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_RAM_PACKET_CONFIG, 0); @@ -762,6 +845,9 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder, vc4_hdmi_disable_scrambling(encoder); + drm_dev_exit(idx); + +out: mutex_unlock(&vc4_hdmi->mutex); } @@ -769,11 +855,16 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; int ret; + int idx; mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) + goto out; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_VID_CTL, HDMI_READ(HDMI_VID_CTL) | VC4_HD_VID_CTL_BLANKPIX); @@ -789,6 +880,9 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder, if (ret < 0) DRM_ERROR("Failed to release power domain: %d\n", ret); + drm_dev_exit(idx); + +out: mutex_unlock(&vc4_hdmi->mutex); } @@ -796,8 +890,13 @@ static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, const struct drm_display_mode *mode) { + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; u32 csc_ctl; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -832,6 +931,8 @@ static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_CSC_CTL, csc_ctl); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + drm_dev_exit(idx); } /* @@ -916,6 +1017,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, const struct drm_display_mode *mode) { + struct drm_device *drm = vc4_hdmi->connector.dev; struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(state); unsigned long flags; @@ -924,6 +1026,10 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, u32 csc_chan_ctl = 0; u32 csc_ctl = VC5_MT_CP_CSC_CTL_ENABLE | VC4_SET_FIELD(VC4_HD_CSC_CTL_MODE_CUSTOM, VC5_MT_CP_CSC_CTL_MODE); + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -966,12 +1072,15 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_CSC_CTL, csc_ctl); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + drm_dev_exit(idx); } static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, struct drm_display_mode *mode) { + struct drm_device *drm = vc4_hdmi->connector.dev; bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC; bool interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE; @@ -991,6 +1100,10 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, VC4_HDMI_VERTB_VBP)); unsigned long flags; u32 reg; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -1023,12 +1136,15 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_MISC_CONTROL, reg); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + drm_dev_exit(idx); } static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, struct drm_display_mode *mode) { + struct drm_device *drm = vc4_hdmi->connector.dev; const struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(state); bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; @@ -1052,6 +1168,10 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, unsigned char gcp; bool gcp_en; u32 reg; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -1128,13 +1248,20 @@ static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, HDMI_WRITE(HDMI_CLOCK_STOP, 0); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + drm_dev_exit(idx); } static void vc4_hdmi_recenter_fifo(struct vc4_hdmi *vc4_hdmi) { + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; u32 drift; int ret; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -1163,12 +1290,15 @@ static void vc4_hdmi_recenter_fifo(struct vc4_hdmi *vc4_hdmi) VC4_HDMI_FIFO_CTL_RECENTER_DONE, 1); WARN_ONCE(ret, "Timeout waiting for " "VC4_HDMI_FIFO_CTL_RECENTER_DONE"); + + drm_dev_exit(idx); } static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_connector *connector = &vc4_hdmi->connector; struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, connector); @@ -1179,9 +1309,13 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, unsigned long bvb_rate, hsm_rate; unsigned long flags; int ret; + int idx; mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) + goto out; + /* * As stated in RPi's vc4 firmware "HDMI state machine (HSM) clock must * be faster than pixel clock, infinitesimally faster, tested in @@ -1202,13 +1336,13 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, ret = clk_set_min_rate(vc4_hdmi->hsm_clock, hsm_rate); if (ret) { DRM_ERROR("Failed to set HSM clock rate: %d\n", ret); - goto out; + goto err_dev_exit; } ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev); if (ret < 0) { DRM_ERROR("Failed to retain power domain: %d\n", ret); - goto out; + goto err_dev_exit; } ret = clk_set_rate(vc4_hdmi->pixel_clock, tmds_char_rate); @@ -1260,6 +1394,8 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, if (vc4_hdmi->variant->set_timings) vc4_hdmi->variant->set_timings(vc4_hdmi, conn_state, mode); + drm_dev_exit(idx); + mutex_unlock(&vc4_hdmi->mutex); return; @@ -1268,6 +1404,8 @@ err_disable_pixel_clock: clk_disable_unprepare(vc4_hdmi->pixel_clock); err_put_runtime_pm: pm_runtime_put(&vc4_hdmi->pdev->dev); +err_dev_exit: + drm_dev_exit(idx); out: mutex_unlock(&vc4_hdmi->mutex); return; @@ -1277,14 +1415,19 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_connector *connector = &vc4_hdmi->connector; struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, connector); unsigned long flags; + int idx; mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) + return; + if (vc4_hdmi->variant->csc_setup) vc4_hdmi->variant->csc_setup(vc4_hdmi, conn_state, mode); @@ -1292,6 +1435,8 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, HDMI_WRITE(HDMI_FIFO_CTL, VC4_HDMI_FIFO_CTL_MASTER_SLAVE_N); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + drm_dev_exit(idx); + mutex_unlock(&vc4_hdmi->mutex); } @@ -1299,15 +1444,20 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, struct drm_atomic_state *state) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; struct drm_display_info *display = &vc4_hdmi->connector.display_info; bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC; unsigned long flags; int ret; + int idx; mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) + return; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_VID_CTL, @@ -1366,6 +1516,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, vc4_hdmi_recenter_fifo(vc4_hdmi); vc4_hdmi_enable_scrambling(encoder); + drm_dev_exit(idx); mutex_unlock(&vc4_hdmi->mutex); } @@ -1714,13 +1865,20 @@ static u32 vc5_hdmi_channel_map(struct vc4_hdmi *vc4_hdmi, u32 channel_mask) static bool vc5_hdmi_hp_detect(struct vc4_hdmi *vc4_hdmi) { + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; u32 hotplug; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return false; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); hotplug = HDMI_READ(HDMI_HOTPLUG); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + drm_dev_exit(idx); + return !!(hotplug & VC4_HDMI_HOTPLUG_CONNECTED); } @@ -1728,10 +1886,16 @@ static bool vc5_hdmi_hp_detect(struct vc4_hdmi *vc4_hdmi) static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *vc4_hdmi, unsigned int samplerate) { - u32 hsm_clock = clk_get_rate(vc4_hdmi->audio_clock); + struct drm_device *drm = vc4_hdmi->connector.dev; + u32 hsm_clock; unsigned long flags; unsigned long n, m; + int idx; + + if (!drm_dev_enter(drm, &idx)) + return; + hsm_clock = clk_get_rate(vc4_hdmi->audio_clock); rational_best_approximation(hsm_clock, samplerate, VC4_HD_MAI_SMP_N_MASK >> VC4_HD_MAI_SMP_N_SHIFT, @@ -1744,6 +1908,8 @@ static void vc4_hdmi_audio_set_mai_clock(struct vc4_hdmi *vc4_hdmi, VC4_SET_FIELD(n, VC4_HD_MAI_SMP_N) | VC4_SET_FIELD(m - 1, VC4_HD_MAI_SMP_M)); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + + drm_dev_exit(idx); } static void vc4_hdmi_set_n_cts(struct vc4_hdmi *vc4_hdmi, unsigned int samplerate) @@ -1799,13 +1965,21 @@ static bool vc4_hdmi_audio_can_stream(struct vc4_hdmi *vc4_hdmi) static int vc4_hdmi_audio_startup(struct device *dev, void *data) { struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int ret = 0; + int idx; mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) { + ret = -ENODEV; + goto out; + } + if (!vc4_hdmi_audio_can_stream(vc4_hdmi)) { - mutex_unlock(&vc4_hdmi->mutex); - return -ENODEV; + ret = -ENODEV; + goto out_dev_exit; } vc4_hdmi->audio.streaming = true; @@ -1822,9 +1996,12 @@ static int vc4_hdmi_audio_startup(struct device *dev, void *data) if (vc4_hdmi->variant->phy_rng_enable) vc4_hdmi->variant->phy_rng_enable(vc4_hdmi); +out_dev_exit: + drm_dev_exit(idx); +out: mutex_unlock(&vc4_hdmi->mutex); - return 0; + return ret; } static void vc4_hdmi_audio_reset(struct vc4_hdmi *vc4_hdmi) @@ -1853,10 +2030,15 @@ static void vc4_hdmi_audio_reset(struct vc4_hdmi *vc4_hdmi) static void vc4_hdmi_audio_shutdown(struct device *dev, void *data) { struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) + goto out; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_MAI_CTL, @@ -1872,6 +2054,9 @@ static void vc4_hdmi_audio_shutdown(struct device *dev, void *data) vc4_hdmi->audio.streaming = false; vc4_hdmi_audio_reset(vc4_hdmi); + drm_dev_exit(idx); + +out: mutex_unlock(&vc4_hdmi->mutex); } @@ -1919,6 +2104,7 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data, struct hdmi_codec_params *params) { struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev); + struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_encoder *encoder = &vc4_hdmi->encoder.base; unsigned int sample_rate = params->sample_rate; unsigned int channels = params->channels; @@ -1927,15 +2113,22 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data, u32 channel_map; u32 mai_audio_format; u32 mai_sample_rate; + int ret = 0; + int idx; dev_dbg(dev, "%s: %u Hz, %d bit, %d channels\n", __func__, sample_rate, params->sample_width, channels); mutex_lock(&vc4_hdmi->mutex); + if (!drm_dev_enter(drm, &idx)) { + ret = -ENODEV; + goto out; + } + if (!vc4_hdmi_audio_can_stream(vc4_hdmi)) { - mutex_unlock(&vc4_hdmi->mutex); - return -EINVAL; + ret = -EINVAL; + goto out_dev_exit; } vc4_hdmi_audio_set_mai_clock(vc4_hdmi, sample_rate); @@ -1992,9 +2185,12 @@ static int vc4_hdmi_audio_prepare(struct device *dev, void *data, memcpy(&vc4_hdmi->audio.infoframe, ¶ms->cea, sizeof(params->cea)); vc4_hdmi_set_audio_infoframe(encoder); +out_dev_exit: + drm_dev_exit(idx); +out: mutex_unlock(&vc4_hdmi->mutex); - return 0; + return ret; } static const struct snd_soc_component_driver vc4_hdmi_audio_cpu_dai_comp = { @@ -2329,6 +2525,17 @@ static irqreturn_t vc4_cec_irq_handler_tx_bare_locked(struct vc4_hdmi *vc4_hdmi) { u32 cntrl1; + /* + * We don't need to protect the register access using + * drm_dev_enter() there because the interrupt handler lifetime + * is tied to the device itself, and not to the DRM device. + * + * So when the device will be gone, one of the first thing we + * will be doing will be to unregister the interrupt handler, + * and then unregister the DRM device. drm_dev_enter() would + * thus always succeed if we are here. + */ + lockdep_assert_held(&vc4_hdmi->hw_lock); cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1); @@ -2357,6 +2564,17 @@ static irqreturn_t vc4_cec_irq_handler_rx_bare_locked(struct vc4_hdmi *vc4_hdmi) lockdep_assert_held(&vc4_hdmi->hw_lock); + /* + * We don't need to protect the register access using + * drm_dev_enter() there because the interrupt handler lifetime + * is tied to the device itself, and not to the DRM device. + * + * So when the device will be gone, one of the first thing we + * will be doing will be to unregister the interrupt handler, + * and then unregister the DRM device. drm_dev_enter() would + * thus always succeed if we are here. + */ + vc4_hdmi->cec_rx_msg.len = 0; cntrl1 = HDMI_READ(HDMI_CEC_CNTRL_1); vc4_cec_read_msg(vc4_hdmi, cntrl1); @@ -2388,6 +2606,17 @@ static irqreturn_t vc4_cec_irq_handler(int irq, void *priv) irqreturn_t ret; u32 cntrl5; + /* + * We don't need to protect the register access using + * drm_dev_enter() there because the interrupt handler lifetime + * is tied to the device itself, and not to the DRM device. + * + * So when the device will be gone, one of the first thing we + * will be doing will be to unregister the interrupt handler, + * and then unregister the DRM device. drm_dev_enter() would + * thus always succeed if we are here. + */ + if (!(stat & VC4_HDMI_CPU_CEC)) return IRQ_NONE; @@ -2408,11 +2637,13 @@ static irqreturn_t vc4_cec_irq_handler(int irq, void *priv) static int vc4_hdmi_cec_enable(struct cec_adapter *adap) { struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap); + struct drm_device *drm = vc4_hdmi->connector.dev; /* clock period in microseconds */ const u32 usecs = 1000000 / CEC_CLOCK_FREQ; unsigned long flags; u32 val; int ret; + int idx; /* * NOTE: This function should really take vc4_hdmi->mutex, but doing so @@ -2425,9 +2656,19 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap) * keep it in mind if we were to change that assumption. */ + if (!drm_dev_enter(drm, &idx)) + /* + * We can't return an error code, because the CEC + * framework will emit WARN_ON messages at unbind + * otherwise. + */ + return 0; + ret = pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev); - if (ret) + if (ret) { + drm_dev_exit(idx); return ret; + } spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -2463,13 +2704,25 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap) spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + drm_dev_exit(idx); + return 0; } static int vc4_hdmi_cec_disable(struct cec_adapter *adap) { struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; + + if (!drm_dev_enter(drm, &idx)) + /* + * We can't return an error code, because the CEC + * framework will emit WARN_ON messages at unbind + * otherwise. + */ + return 0; /* * NOTE: This function should really take vc4_hdmi->mutex, but doing so @@ -2494,6 +2747,8 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap) pm_runtime_put(&vc4_hdmi->pdev->dev); + drm_dev_exit(idx); + return 0; } @@ -2508,7 +2763,9 @@ static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable) static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) { struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap); + struct drm_device *drm = vc4_hdmi->connector.dev; unsigned long flags; + int idx; /* * NOTE: This function should really take vc4_hdmi->mutex, but doing so @@ -2521,12 +2778,22 @@ static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) * keep it in mind if we were to change that assumption. */ + if (!drm_dev_enter(drm, &idx)) + /* + * We can't return an error code, because the CEC + * framework will emit WARN_ON messages at unbind + * otherwise. + */ + return 0; + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_CEC_CNTRL_1, (HDMI_READ(HDMI_CEC_CNTRL_1) & ~VC4_HDMI_CEC_ADDR_MASK) | (log_addr & 0xf) << VC4_HDMI_CEC_ADDR_SHIFT); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + drm_dev_exit(idx); + return 0; } @@ -2538,6 +2805,7 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, unsigned long flags; u32 val; unsigned int i; + int idx; /* * NOTE: This function should really take vc4_hdmi->mutex, but doing so @@ -2550,8 +2818,12 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, * keep it in mind if we were to change that assumption. */ + if (!drm_dev_enter(dev, &idx)) + return -ENODEV; + if (msg->len > 16) { drm_err(dev, "Attempting to transmit too much data (%d)\n", msg->len); + drm_dev_exit(idx); return -ENOMEM; } @@ -2575,6 +2847,8 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + drm_dev_exit(idx); + return 0; } -- cgit v1.2.3 From 932d860f46722cd91a9d012c94b2d32013a8ea15 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:18 +0200 Subject: drm/vc4: hdmi: Switch to devm_pm_runtime_enable devm_pm_runtime_enable() simplifies the driver a bit since it will call pm_runtime_disable() automatically through a device-managed action. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-49-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index a826faf8b02d..166d22d62d99 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -3314,7 +3314,9 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) vc4_hdmi->disable_4kp60 = true; } - pm_runtime_enable(dev); + ret = devm_pm_runtime_enable(dev); + if (ret) + return ret; /* * We need to have the device powered up at this point to call @@ -3322,7 +3324,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) */ ret = pm_runtime_resume_and_get(dev); if (ret) - goto err_disable_runtime_pm; + return ret; if ((of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi0") || of_device_is_compatible(dev->of_node, "brcm,bcm2711-hdmi1")) && @@ -3367,21 +3369,12 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) err_put_runtime_pm: pm_runtime_put_sync(dev); -err_disable_runtime_pm: - pm_runtime_disable(dev); return ret; } -static void vc4_hdmi_unbind(struct device *dev, struct device *master, - void *data) -{ - pm_runtime_disable(dev); -} - static const struct component_ops vc4_hdmi_ops = { .bind = vc4_hdmi_bind, - .unbind = vc4_hdmi_unbind, }; static int vc4_hdmi_dev_probe(struct platform_device *pdev) -- cgit v1.2.3 From 445b287e18ca374b5498e0f7cecb6c5f673c4edb Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:34 +0200 Subject: drm/vc4: debugfs: Simplify debugfs registration The vc4 has a custom API to allow components to register a debugfs file before the DRM driver has been registered and the debugfs_init hook has been called. However, the .late_register hook allows to have the debugfs file creation deferred after that time already. Let's remove our custom code to only register later our debugfs entries as part of either debugfs_init or after it. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-65-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_bo.c | 20 ++++++++++++--- drivers/gpu/drm/vc4/vc4_crtc.c | 19 ++++++++++++--- drivers/gpu/drm/vc4/vc4_debugfs.c | 51 +++++++++++++-------------------------- drivers/gpu/drm/vc4/vc4_dpi.c | 21 +++++++++++++--- drivers/gpu/drm/vc4/vc4_drv.h | 12 ++++++--- drivers/gpu/drm/vc4/vc4_dsi.c | 22 ++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.c | 26 ++++++++++++++++---- drivers/gpu/drm/vc4/vc4_hvs.c | 39 +++++++++++++++++++++++++----- drivers/gpu/drm/vc4/vc4_txp.c | 3 +-- drivers/gpu/drm/vc4/vc4_v3d.c | 25 ++++++++++++++++--- drivers/gpu/drm/vc4/vc4_vec.c | 22 ++++++++++++++--- 11 files changed, 191 insertions(+), 69 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 0846d56f74f2..9706fec47bcd 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -984,6 +984,23 @@ int vc4_get_tiling_ioctl(struct drm_device *dev, void *data, return 0; } +int vc4_bo_debugfs_init(struct drm_minor *minor) +{ + struct drm_device *drm = minor->dev; + struct vc4_dev *vc4 = to_vc4_dev(drm); + int ret; + + if (!vc4->v3d) + return -ENODEV; + + ret = vc4_debugfs_add_file(minor, "bo_stats", + vc4_bo_stats_debugfs, NULL); + if (ret) + return ret; + + return 0; +} + static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused); int vc4_bo_cache_init(struct drm_device *dev) { @@ -1008,9 +1025,6 @@ int vc4_bo_cache_init(struct drm_device *dev) vc4->bo_labels[i].name = bo_type_names[i]; mutex_init(&vc4->bo_lock); - - vc4_debugfs_add_file(dev, "bo_stats", vc4_bo_stats_debugfs, NULL); - INIT_LIST_HEAD(&vc4->bo_cache.time_list); INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work); diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 3768a2a57ca9..d3efa7e5b5ed 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -1052,6 +1052,21 @@ void vc4_crtc_reset(struct drm_crtc *crtc) __drm_atomic_helper_crtc_reset(crtc, &vc4_crtc_state->base); } +int vc4_crtc_late_register(struct drm_crtc *crtc) +{ + struct drm_device *drm = crtc->dev; + struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); + const struct vc4_crtc_data *crtc_data = vc4_crtc_to_vc4_crtc_data(vc4_crtc); + int ret; + + ret = vc4_debugfs_add_regset32(drm->primary, crtc_data->debugfs_name, + &vc4_crtc->regset); + if (ret) + return ret; + + return 0; +} + static const struct drm_crtc_funcs vc4_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .page_flip = vc4_page_flip, @@ -1064,6 +1079,7 @@ static const struct drm_crtc_funcs vc4_crtc_funcs = { .enable_vblank = vc4_enable_vblank, .disable_vblank = vc4_disable_vblank, .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp, + .late_register = vc4_crtc_late_register, }; static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = { @@ -1318,9 +1334,6 @@ 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->base.debugfs_name, - &vc4_crtc->regset); - return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c index b857fb9c94bc..19cda4f91a82 100644 --- a/drivers/gpu/drm/vc4/vc4_debugfs.c +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c @@ -14,11 +14,6 @@ #include "vc4_drv.h" #include "vc4_regs.h" -struct vc4_debugfs_info_entry { - struct list_head link; - struct drm_info_list info; -}; - /* * Called at drm_dev_register() time on each of the minors registered * by the DRM device, to attach the debugfs files. @@ -27,16 +22,13 @@ void vc4_debugfs_init(struct drm_minor *minor) { struct vc4_dev *vc4 = to_vc4_dev(minor->dev); - struct vc4_debugfs_info_entry *entry; + struct drm_device *drm = &vc4->base; - if (!of_device_is_compatible(vc4->hvs->pdev->dev.of_node, - "brcm,bcm2711-vc5")) - debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR, - minor->debugfs_root, &vc4->load_tracker_enabled); + drm_WARN_ON(drm, vc4_hvs_debugfs_init(minor)); - list_for_each_entry(entry, &vc4->debugfs_list, link) { - drm_debugfs_create_files(&entry->info, 1, - minor->debugfs_root, minor); + if (vc4->v3d) { + drm_WARN_ON(drm, vc4_bo_debugfs_init(minor)); + drm_WARN_ON(drm, vc4_v3d_debugfs_init(minor)); } } @@ -58,40 +50,31 @@ static int vc4_debugfs_regset32(struct seq_file *m, void *unused) return 0; } -/* - * Registers a debugfs file with a callback function for a vc4 component. - * - * This is like drm_debugfs_create_files(), but that can only be - * called a given DRM minor, while the various VC4 components want to - * register their debugfs files during the component bind process. We - * track the request and delay it to be called on each minor during - * vc4_debugfs_init(). - */ -int vc4_debugfs_add_file(struct drm_device *dev, +int vc4_debugfs_add_file(struct drm_minor *minor, const char *name, int (*show)(struct seq_file*, void*), void *data) { - struct vc4_dev *vc4 = to_vc4_dev(dev); - - struct vc4_debugfs_info_entry *entry = - devm_kzalloc(dev->dev, sizeof(*entry), GFP_KERNEL); + struct drm_device *dev = minor->dev; + struct dentry *root = minor->debugfs_root; + struct drm_info_list *file; - if (!entry) + file = drmm_kzalloc(dev, sizeof(*file), GFP_KERNEL); + if (!file) return -ENOMEM; - entry->info.name = name; - entry->info.show = show; - entry->info.data = data; + file->name = name; + file->show = show; + file->data = data; - list_add(&entry->link, &vc4->debugfs_list); + drm_debugfs_create_files(file, 1, root, minor); return 0; } -int vc4_debugfs_add_regset32(struct drm_device *drm, +int vc4_debugfs_add_regset32(struct drm_minor *minor, const char *name, struct debugfs_regset32 *regset) { - return vc4_debugfs_add_file(drm, name, vc4_debugfs_regset32, regset); + return vc4_debugfs_add_file(minor, name, vc4_debugfs_regset32, regset); } diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index 7286a3333538..1f8f44b7b5a5 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -244,6 +244,23 @@ static const struct drm_encoder_helper_funcs vc4_dpi_encoder_helper_funcs = { .mode_valid = vc4_dpi_encoder_mode_valid, }; +static int vc4_dpi_late_register(struct drm_encoder *encoder) +{ + struct drm_device *drm = encoder->dev; + struct vc4_dpi *dpi = to_vc4_dpi(encoder); + int ret; + + ret = vc4_debugfs_add_regset32(drm->primary, "dpi_regs", &dpi->regset); + if (ret) + return ret; + + return 0; +} + +static const struct drm_encoder_funcs vc4_dpi_encoder_funcs = { + .late_register = vc4_dpi_late_register, +}; + static const struct of_device_id vc4_dpi_dt_match[] = { { .compatible = "brcm,bcm2835-dpi", .data = NULL }, {} @@ -332,7 +349,7 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) return ret; ret = drmm_encoder_init(drm, &dpi->encoder.base, - NULL, + &vc4_dpi_encoder_funcs, DRM_MODE_ENCODER_DPI, NULL); if (ret) @@ -346,8 +363,6 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) dev_set_drvdata(dev, dpi); - vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset); - return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 5d857816426b..1649350b9efd 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -841,6 +841,7 @@ int vc4_bo_inc_usecnt(struct vc4_bo *bo); void vc4_bo_dec_usecnt(struct vc4_bo *bo); void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo); void vc4_bo_remove_from_purgeable_pool(struct vc4_bo *bo); +int vc4_bo_debugfs_init(struct drm_minor *minor); /* vc4_crtc.c */ extern struct platform_driver vc4_crtc_driver; @@ -859,6 +860,7 @@ void vc4_crtc_destroy_state(struct drm_crtc *crtc, void vc4_crtc_reset(struct drm_crtc *crtc); void vc4_crtc_handle_vblank(struct vc4_crtc *crtc); void vc4_crtc_send_vblank(struct drm_crtc *crtc); +int vc4_crtc_late_register(struct drm_crtc *crtc); void vc4_crtc_get_margins(struct drm_crtc_state *state, unsigned int *left, unsigned int *right, unsigned int *top, unsigned int *bottom); @@ -866,15 +868,15 @@ void vc4_crtc_get_margins(struct drm_crtc_state *state, /* vc4_debugfs.c */ void vc4_debugfs_init(struct drm_minor *minor); #ifdef CONFIG_DEBUG_FS -int vc4_debugfs_add_file(struct drm_device *drm, +int vc4_debugfs_add_file(struct drm_minor *minor, const char *filename, int (*show)(struct seq_file*, void*), void *data); -int vc4_debugfs_add_regset32(struct drm_device *drm, +int vc4_debugfs_add_regset32(struct drm_minor *minor, const char *filename, struct debugfs_regset32 *regset); #else -static inline int vc4_debugfs_add_file(struct drm_device *drm, +static inline int vc4_debugfs_add_file(struct drm_minor *minor, const char *filename, int (*show)(struct seq_file*, void*), void *data) @@ -882,7 +884,7 @@ static inline int vc4_debugfs_add_file(struct drm_device *drm, return 0; } -static inline int vc4_debugfs_add_regset32(struct drm_device *drm, +static inline int vc4_debugfs_add_regset32(struct drm_minor *minor, const char *filename, struct debugfs_regset32 *regset) { @@ -952,6 +954,7 @@ void vc4_hvs_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *state) void vc4_hvs_dump_state(struct vc4_hvs *hvs); void vc4_hvs_unmask_underrun(struct vc4_hvs *hvs, int channel); void vc4_hvs_mask_underrun(struct vc4_hvs *hvs, int channel); +int vc4_hvs_debugfs_init(struct drm_minor *minor); /* vc4_kms.c */ int vc4_kms_load(struct drm_device *dev); @@ -974,6 +977,7 @@ int vc4_v3d_bin_bo_get(struct vc4_dev *vc4, bool *used); void vc4_v3d_bin_bo_put(struct vc4_dev *vc4); int vc4_v3d_pm_get(struct vc4_dev *vc4); void vc4_v3d_pm_put(struct vc4_dev *vc4); +int vc4_v3d_debugfs_init(struct drm_minor *minor); /* vc4_validate.c */ int diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 52c3215fef49..878e05d79e81 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1372,6 +1372,24 @@ static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = { .mode_fixup = vc4_dsi_encoder_mode_fixup, }; +static int vc4_dsi_late_register(struct drm_encoder *encoder) +{ + struct drm_device *drm = encoder->dev; + struct vc4_dsi *dsi = to_vc4_dsi(encoder); + int ret; + + ret = vc4_debugfs_add_regset32(drm->primary, dsi->variant->debugfs_name, + &dsi->regset); + if (ret) + return ret; + + return 0; +} + +static const struct drm_encoder_funcs vc4_dsi_encoder_funcs = { + .late_register = vc4_dsi_late_register, +}; + static const struct vc4_dsi_variant bcm2711_dsi1_variant = { .port = 1, .debugfs_name = "dsi1_regs", @@ -1721,7 +1739,7 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return ret; ret = drmm_encoder_init(drm, encoder, - NULL, + &vc4_dsi_encoder_funcs, DRM_MODE_ENCODER_DSI, NULL); if (ret) @@ -1743,8 +1761,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) */ list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain); - vc4_debugfs_add_regset32(drm, dsi->variant->debugfs_name, &dsi->regset); - return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 166d22d62d99..4acc865576f0 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1839,6 +1839,26 @@ static const struct drm_encoder_helper_funcs vc4_hdmi_encoder_helper_funcs = { .mode_valid = vc4_hdmi_encoder_mode_valid, }; +static int vc4_hdmi_late_register(struct drm_encoder *encoder) +{ + struct drm_device *drm = encoder->dev; + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + const struct vc4_hdmi_variant *variant = vc4_hdmi->variant; + int ret; + + ret = vc4_debugfs_add_file(drm->primary, variant->debugfs_name, + vc4_hdmi_debugfs_regs, + vc4_hdmi); + if (ret) + return ret; + + return 0; +} + +static const struct drm_encoder_funcs vc4_hdmi_encoder_funcs = { + .late_register = vc4_hdmi_late_register, +}; + static u32 vc4_hdmi_channel_map(struct vc4_hdmi *vc4_hdmi, u32 channel_mask) { int i; @@ -3335,7 +3355,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) } ret = drmm_encoder_init(drm, encoder, - NULL, + &vc4_hdmi_encoder_funcs, DRM_MODE_ENCODER_TMDS, NULL); if (ret) @@ -3359,10 +3379,6 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_put_runtime_pm; - vc4_debugfs_add_file(drm, variant->debugfs_name, - vc4_hdmi_debugfs_regs, - vc4_hdmi); - pm_runtime_put_sync(dev); return 0; diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c index a62f222255ce..9e823e0de197 100644 --- a/drivers/gpu/drm/vc4/vc4_hvs.c +++ b/drivers/gpu/drm/vc4/vc4_hvs.c @@ -733,6 +733,39 @@ static irqreturn_t vc4_hvs_irq_handler(int irq, void *data) return irqret; } +int vc4_hvs_debugfs_init(struct drm_minor *minor) +{ + struct drm_device *drm = minor->dev; + struct vc4_dev *vc4 = to_vc4_dev(drm); + struct vc4_hvs *hvs = vc4->hvs; + int ret; + + if (!vc4->hvs) + return -ENODEV; + + if (!vc4->is_vc5) + debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR, + minor->debugfs_root, + &vc4->load_tracker_enabled); + + ret = vc4_debugfs_add_file(minor, "hvs_dlists", + vc4_hvs_debugfs_dlist, NULL); + if (ret) + return ret; + + ret = vc4_debugfs_add_file(minor, "hvs_underrun", + vc4_hvs_debugfs_underrun, NULL); + if (ret) + return ret; + + ret = vc4_debugfs_add_regset32(minor, "hvs_regs", + &hvs->regset); + if (ret) + return ret; + + return 0; +} + static int vc4_hvs_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); @@ -858,12 +891,6 @@ static int vc4_hvs_bind(struct device *dev, struct device *master, void *data) if (ret) return ret; - vc4_debugfs_add_regset32(drm, "hvs_regs", &hvs->regset); - vc4_debugfs_add_file(drm, "hvs_underrun", vc4_hvs_debugfs_underrun, - NULL); - vc4_debugfs_add_file(drm, "hvs_dlists", vc4_hvs_debugfs_dlist, - NULL); - return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index f6f61cf5f3f7..913e26dd62d4 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -398,6 +398,7 @@ static const struct drm_crtc_funcs vc4_txp_crtc_funcs = { .atomic_destroy_state = vc4_crtc_destroy_state, .enable_vblank = vc4_txp_enable_vblank, .disable_vblank = vc4_txp_disable_vblank, + .late_register = vc4_crtc_late_register, }; static int vc4_txp_atomic_check(struct drm_crtc *crtc, @@ -541,8 +542,6 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) dev_set_drvdata(dev, txp); - vc4_debugfs_add_regset32(drm, "txp_regs", &vc4_crtc->regset); - return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c index cc714dcfe1f2..e7990bed0a96 100644 --- a/drivers/gpu/drm/vc4/vc4_v3d.c +++ b/drivers/gpu/drm/vc4/vc4_v3d.c @@ -401,6 +401,28 @@ static int vc4_v3d_runtime_resume(struct device *dev) } #endif +int vc4_v3d_debugfs_init(struct drm_minor *minor) +{ + struct drm_device *drm = minor->dev; + struct vc4_dev *vc4 = to_vc4_dev(drm); + struct vc4_v3d *v3d = vc4->v3d; + int ret; + + if (!vc4->v3d) + return -ENODEV; + + ret = vc4_debugfs_add_file(minor, "v3d_ident", + vc4_v3d_debugfs_ident, NULL); + if (ret) + return ret; + + ret = vc4_debugfs_add_regset32(minor, "v3d_regs", &v3d->regset); + if (ret) + return ret; + + return 0; +} + static int vc4_v3d_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); @@ -477,9 +499,6 @@ static int vc4_v3d_bind(struct device *dev, struct device *master, void *data) pm_runtime_set_autosuspend_delay(dev, 40); /* a little over 2 frames. */ pm_runtime_enable(dev); - vc4_debugfs_add_file(drm, "v3d_ident", vc4_v3d_debugfs_ident, NULL); - vc4_debugfs_add_regset32(drm, "v3d_regs", &v3d->regset); - return 0; } diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c index d356ffa52866..4a788c1c9058 100644 --- a/drivers/gpu/drm/vc4/vc4_vec.c +++ b/drivers/gpu/drm/vc4/vc4_vec.c @@ -523,6 +523,24 @@ static const struct drm_encoder_helper_funcs vc4_vec_encoder_helper_funcs = { .atomic_mode_set = vc4_vec_encoder_atomic_mode_set, }; +static int vc4_vec_late_register(struct drm_encoder *encoder) +{ + struct drm_device *drm = encoder->dev; + struct vc4_vec *vec = encoder_to_vc4_vec(encoder); + int ret; + + ret = vc4_debugfs_add_regset32(drm->primary, "vec_regs", + &vec->regset); + if (ret) + return ret; + + return 0; +} + +static const struct drm_encoder_funcs vc4_vec_encoder_funcs = { + .late_register = vc4_vec_late_register, +}; + static const struct vc4_vec_variant bcm2835_vec_variant = { .dac_config = VEC_DAC_CONFIG_DAC_CTRL(0xc) | VEC_DAC_CONFIG_DRIVER_CTRL(0xc) | @@ -588,7 +606,7 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data) return ret; ret = drmm_encoder_init(drm, &vec->encoder.base, - NULL, + &vc4_vec_encoder_funcs, DRM_MODE_ENCODER_TVDAC, NULL); if (ret) @@ -602,8 +620,6 @@ static int vc4_vec_bind(struct device *dev, struct device *master, void *data) dev_set_drvdata(dev, vec); - vc4_debugfs_add_regset32(drm, "vec_regs", &vec->regset); - return 0; } -- cgit v1.2.3 From 374146cad469102b683939009f18fa88f9b9b6c9 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 11 Jul 2022 19:39:35 +0200 Subject: drm/vc4: Switch to drmm_mutex_init mutex_init is supposed to be balanced by a call to mutex_destroy that we were never doing in the vc4 driver. Since a DRM-managed mutex_init variant has been introduced, let's just switch to it. Acked-by: Thomas Zimmermann Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220711173939.1132294-66-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_bo.c | 15 +++++++++++++-- drivers/gpu/drm/vc4/vc4_drv.c | 4 +++- drivers/gpu/drm/vc4/vc4_gem.c | 10 ++++++++-- drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++++- 4 files changed, 28 insertions(+), 6 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c index 9706fec47bcd..2ccf96b764db 100644 --- a/drivers/gpu/drm/vc4/vc4_bo.c +++ b/drivers/gpu/drm/vc4/vc4_bo.c @@ -394,6 +394,7 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size) { struct vc4_dev *vc4 = to_vc4_dev(dev); struct vc4_bo *bo; + int ret; if (WARN_ON_ONCE(vc4->is_vc5)) return ERR_PTR(-ENODEV); @@ -404,7 +405,11 @@ struct drm_gem_object *vc4_create_object(struct drm_device *dev, size_t size) bo->madv = VC4_MADV_WILLNEED; refcount_set(&bo->usecnt, 0); - mutex_init(&bo->madv_lock); + + ret = drmm_mutex_init(dev, &bo->madv_lock); + if (ret) + return ERR_PTR(ret); + mutex_lock(&vc4->bo_lock); bo->label = VC4_BO_TYPE_KERNEL; vc4->bo_labels[VC4_BO_TYPE_KERNEL].num_allocated++; @@ -1005,6 +1010,7 @@ static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused); int vc4_bo_cache_init(struct drm_device *dev) { struct vc4_dev *vc4 = to_vc4_dev(dev); + int ret; int i; if (WARN_ON_ONCE(vc4->is_vc5)) @@ -1024,7 +1030,12 @@ int vc4_bo_cache_init(struct drm_device *dev) for (i = 0; i < VC4_BO_TYPE_COUNT; i++) vc4->bo_labels[i].name = bo_type_names[i]; - mutex_init(&vc4->bo_lock); + ret = drmm_mutex_init(dev, &vc4->bo_lock); + if (ret) { + kfree(vc4->bo_labels); + return ret; + } + INIT_LIST_HEAD(&vc4->bo_cache.time_list); INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work); diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 79a37d18aeb1..89975bdd607e 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -324,7 +324,9 @@ static int vc4_drm_bind(struct device *dev) INIT_LIST_HEAD(&vc4->debugfs_list); if (!is_vc5) { - mutex_init(&vc4->bin_bo_lock); + ret = drmm_mutex_init(drm, &vc4->bin_bo_lock); + if (ret) + return ret; ret = vc4_bo_cache_init(drm); if (ret) diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index fe10d9c3fff8..7acb43972e69 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -1308,6 +1308,7 @@ static void vc4_gem_destroy(struct drm_device *dev, void *unused); int vc4_gem_init(struct drm_device *dev) { struct vc4_dev *vc4 = to_vc4_dev(dev); + int ret; if (WARN_ON_ONCE(vc4->is_vc5)) return -ENODEV; @@ -1325,10 +1326,15 @@ int vc4_gem_init(struct drm_device *dev) INIT_WORK(&vc4->job_done_work, vc4_job_done_work); - mutex_init(&vc4->power_lock); + ret = drmm_mutex_init(dev, &vc4->power_lock); + if (ret) + return ret; INIT_LIST_HEAD(&vc4->purgeable.list); - mutex_init(&vc4->purgeable.lock); + + ret = drmm_mutex_init(dev, &vc4->purgeable.lock); + if (ret) + return ret; return drmm_add_action_or_reset(dev, vc4_gem_destroy, NULL); } diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4acc865576f0..6f61a1b8a1a3 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -3270,7 +3270,10 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data) if (!vc4_hdmi) return -ENOMEM; - mutex_init(&vc4_hdmi->mutex); + ret = drmm_mutex_init(drm, &vc4_hdmi->mutex); + if (ret) + return ret; + spin_lock_init(&vc4_hdmi->hw_lock); INIT_DELAYED_WORK(&vc4_hdmi->scrambling_work, vc4_hdmi_scrambling_wq); -- cgit v1.2.3 From 3b3510f1bd5c3112811b5b79cae3754fc91d31b0 Mon Sep 17 00:00:00 2001 From: Maíra Canal Date: Fri, 12 Aug 2022 17:57:46 -0300 Subject: drm/vc4: Drop of_gpio header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This driver includes the deprecated OF GPIO header yet fail to use symbols from it, so drop the include. Cc: Emma Anholt Cc: Maxime Ripard Signed-off-by: Maíra Canal Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220812205746.609107-6-mairacanal@riseup.net --- drivers/gpu/drm/vc4/vc4_hdmi.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 6f61a1b8a1a3..84e5a91c2ea7 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include -- cgit v1.2.3 From 227295df4e37de66b61bbb3d1f10436f0acd33cc Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 24 Aug 2022 18:13:24 +0200 Subject: drm/vc4: hdmi: unlock mutex when device is unplugged In vc4_hdmi_encoder_{pre,post}_crtc_enable() commit cd00ed5187bf ("drm/vc4: hdmi: Protect device resources after removal") missed to unlock the mutex before returning due to drm_dev_enter() indicating the device being unplugged. Fixes: cd00ed5187bf ("drm/vc4: hdmi: Protect device resources after removal") Signed-off-by: Danilo Krummrich Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220824161327.330627-2-dakr@redhat.com --- drivers/gpu/drm/vc4/vc4_hdmi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 84e5a91c2ea7..4d3ff51ad2a8 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -1425,7 +1425,7 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, mutex_lock(&vc4_hdmi->mutex); if (!drm_dev_enter(drm, &idx)) - return; + goto out; if (vc4_hdmi->variant->csc_setup) vc4_hdmi->variant->csc_setup(vc4_hdmi, conn_state, mode); @@ -1436,6 +1436,7 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, drm_dev_exit(idx); +out: mutex_unlock(&vc4_hdmi->mutex); } @@ -1455,7 +1456,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, mutex_lock(&vc4_hdmi->mutex); if (!drm_dev_enter(drm, &idx)) - return; + goto out; spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -1516,6 +1517,8 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, vc4_hdmi_enable_scrambling(encoder); drm_dev_exit(idx); + +out: mutex_unlock(&vc4_hdmi->mutex); } -- cgit v1.2.3 From 58009c256b1134a5257407f60fd37396dc99cbca Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:24 +0200 Subject: drm/vc4: hdmi: Constify drm_display_mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't modify the drm_display_mode pointer we have in the driver in most places, so let's make them const. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-2-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 16 ++++++++-------- drivers/gpu/drm/vc4/vc4_hdmi.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4d3ff51ad2a8..773176d810a4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -335,7 +335,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) if (vc4_hdmi->disable_4kp60) { struct drm_device *drm = connector->dev; - struct drm_display_mode *mode; + const struct drm_display_mode *mode; list_for_each_entry(mode, &connector->probed_modes, head) { if (vc4_hdmi_mode_needs_scrambling(mode, 8, VC4_HDMI_OUTPUT_RGB)) { @@ -709,7 +709,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) } static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder, - struct drm_display_mode *mode) + const struct drm_display_mode *mode) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); struct drm_display_info *display = &vc4_hdmi->connector.display_info; @@ -732,7 +732,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); struct drm_device *drm = vc4_hdmi->connector.dev; - struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; + const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; unsigned long flags; int idx; @@ -1077,7 +1077,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi, static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, - struct drm_display_mode *mode) + const struct drm_display_mode *mode) { struct drm_device *drm = vc4_hdmi->connector.dev; bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; @@ -1141,7 +1141,7 @@ static void vc4_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, static void vc5_hdmi_set_timings(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, - struct drm_display_mode *mode) + const struct drm_display_mode *mode) { struct drm_device *drm = vc4_hdmi->connector.dev; const struct vc4_hdmi_connector_state *vc4_state = @@ -1303,7 +1303,7 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder, drm_atomic_get_new_connector_state(state, connector); struct vc4_hdmi_connector_state *vc4_conn_state = conn_state_to_vc4_hdmi_conn_state(conn_state); - struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; + const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; unsigned long tmds_char_rate = vc4_conn_state->tmds_char_rate; unsigned long bvb_rate, hsm_rate; unsigned long flags; @@ -1416,7 +1416,7 @@ static void vc4_hdmi_encoder_pre_crtc_enable(struct drm_encoder *encoder, struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); struct drm_device *drm = vc4_hdmi->connector.dev; struct drm_connector *connector = &vc4_hdmi->connector; - struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; + const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(state, connector); unsigned long flags; @@ -1445,7 +1445,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder, { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); struct drm_device *drm = vc4_hdmi->connector.dev; - struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; + const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; struct drm_display_info *display = &vc4_hdmi->connector.display_info; bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC; bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 99b0bc1297be..79495ef79f09 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -72,7 +72,7 @@ struct vc4_hdmi_variant { /* Callback to configure the video timings in the HDMI block */ void (*set_timings)(struct vc4_hdmi *vc4_hdmi, struct drm_connector_state *state, - struct drm_display_mode *mode); + const struct drm_display_mode *mode); /* Callback to initialize the PHY according to the connector state */ void (*phy_init)(struct vc4_hdmi *vc4_hdmi, -- cgit v1.2.3 From d098a31fe0f867cb7c00a4206740e768b00d8550 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:25 +0200 Subject: drm/vc4: hdmi: Remove unused argument in vc4_hdmi_supports_scrambling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though vc4_hdmi_supports_scrambling takes a mode as an argument, it never uses it. Let's remove it. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-3-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 773176d810a4..d33059865994 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -708,8 +708,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_hdr_infoframe(encoder); } -static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder, - const struct drm_display_mode *mode) +static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); struct drm_display_info *display = &vc4_hdmi->connector.display_info; @@ -738,7 +737,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) lockdep_assert_held(&vc4_hdmi->mutex); - if (!vc4_hdmi_supports_scrambling(encoder, mode)) + if (!vc4_hdmi_supports_scrambling(encoder)) return; if (!vc4_hdmi_mode_needs_scrambling(mode, -- cgit v1.2.3 From da94e9c64c19a7fb2caa896628cb501a06cc3e96 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:26 +0200 Subject: drm/vc4: hdmi: Remove mutex in detect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We recently introduced a new mutex to protect concurrent execution of ALSA and KMS hooks, and the concurrent access to some of vc4_hdmi fields. However, using it in the detect hook was creating a reentrency issue with CEC code. Indeed, calling cec_s_phys_addr_from_edid from detect might call the CEC adap_enable hook with the lock held, eventually resulting in a deadlock. Since we didn't really need to protect anything at the moment in the CEC code, the decision was made to ignore the mutex in those CEC hooks, working around the issue. However, we can have the same thing happening if we end up triggering a mode set from the detect callback, for example using drm_atomic_helper_connector_hdmi_reset_link(). Since we don't really need to protect anything in detect either, let's just drop the lock in detect, and add it again in CEC. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-4-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 88 ++++++++++++++++-------------------------- drivers/gpu/drm/vc4/vc4_hdmi.h | 10 +---- 2 files changed, 34 insertions(+), 64 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index d33059865994..308f7440eb6a 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -279,7 +279,16 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); bool connected = false; - mutex_lock(&vc4_hdmi->mutex); + /* + * NOTE: This function should really take vc4_hdmi->mutex, but + * doing so results in reentrancy issues since + * cec_s_phys_addr_from_edid might call .adap_enable, which + * leads to that funtion being called with our mutex held. + * + * Concurrency isn't an issue at the moment since we don't share + * any state with any of the other frameworks so we can ignore + * the lock for now. + */ WARN_ON(pm_runtime_resume_and_get(&vc4_hdmi->pdev->dev)); @@ -304,13 +313,11 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base); pm_runtime_put(&vc4_hdmi->pdev->dev); - mutex_unlock(&vc4_hdmi->mutex); return connector_status_connected; } cec_phys_addr_invalidate(vc4_hdmi->cec_adap); pm_runtime_put(&vc4_hdmi->pdev->dev); - mutex_unlock(&vc4_hdmi->mutex); return connector_status_disconnected; } @@ -320,14 +327,21 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) int ret = 0; struct edid *edid; - mutex_lock(&vc4_hdmi->mutex); + /* + * NOTE: This function should really take vc4_hdmi->mutex, but + * doing so results in reentrancy issues since + * cec_s_phys_addr_from_edid might call .adap_enable, which + * leads to that funtion being called with our mutex held. + * + * Concurrency isn't an issue at the moment since we don't share + * any state with any of the other frameworks so we can ignore + * the lock for now. + */ edid = drm_get_edid(connector, vc4_hdmi->ddc); cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); - if (!edid) { - ret = -ENODEV; - goto out; - } + if (!edid) + return -ENODEV; drm_connector_update_edid_property(connector, edid); ret = drm_add_edid_modes(connector, edid); @@ -345,9 +359,6 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) } } -out: - mutex_unlock(&vc4_hdmi->mutex); - return ret; } @@ -2666,17 +2677,6 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap) int ret; int idx; - /* - * NOTE: This function should really take vc4_hdmi->mutex, but doing so - * results in a reentrancy since cec_s_phys_addr_from_edid() called in - * .detect or .get_modes might call .adap_enable, which leads to this - * function being called with that mutex held. - * - * Concurrency is not an issue for the moment since we don't share any - * state with KMS, so we can ignore the lock for now, but we need to - * keep it in mind if we were to change that assumption. - */ - if (!drm_dev_enter(drm, &idx)) /* * We can't return an error code, because the CEC @@ -2691,6 +2691,8 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap) return ret; } + mutex_lock(&vc4_hdmi->mutex); + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); val = HDMI_READ(HDMI_CEC_CNTRL_5); @@ -2725,6 +2727,7 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap) spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + mutex_unlock(&vc4_hdmi->mutex); drm_dev_exit(idx); return 0; @@ -2745,16 +2748,7 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap) */ return 0; - /* - * NOTE: This function should really take vc4_hdmi->mutex, but doing so - * results in a reentrancy since cec_s_phys_addr_from_edid() called in - * .detect or .get_modes might call .adap_enable, which leads to this - * function being called with that mutex held. - * - * Concurrency is not an issue for the moment since we don't share any - * state with KMS, so we can ignore the lock for now, but we need to - * keep it in mind if we were to change that assumption. - */ + mutex_lock(&vc4_hdmi->mutex); spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); @@ -2766,6 +2760,8 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap) spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + mutex_unlock(&vc4_hdmi->mutex); + pm_runtime_put(&vc4_hdmi->pdev->dev); drm_dev_exit(idx); @@ -2788,17 +2784,6 @@ static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) unsigned long flags; int idx; - /* - * NOTE: This function should really take vc4_hdmi->mutex, but doing so - * results in a reentrancy since cec_s_phys_addr_from_edid() called in - * .detect or .get_modes might call .adap_enable, which leads to this - * function being called with that mutex held. - * - * Concurrency is not an issue for the moment since we don't share any - * state with KMS, so we can ignore the lock for now, but we need to - * keep it in mind if we were to change that assumption. - */ - if (!drm_dev_enter(drm, &idx)) /* * We can't return an error code, because the CEC @@ -2807,11 +2792,13 @@ static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) */ return 0; + mutex_lock(&vc4_hdmi->mutex); spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); HDMI_WRITE(HDMI_CEC_CNTRL_1, (HDMI_READ(HDMI_CEC_CNTRL_1) & ~VC4_HDMI_CEC_ADDR_MASK) | (log_addr & 0xf) << VC4_HDMI_CEC_ADDR_SHIFT); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); + mutex_unlock(&vc4_hdmi->mutex); drm_dev_exit(idx); @@ -2828,17 +2815,6 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, unsigned int i; int idx; - /* - * NOTE: This function should really take vc4_hdmi->mutex, but doing so - * results in a reentrancy since cec_s_phys_addr_from_edid() called in - * .detect or .get_modes might call .adap_enable, which leads to this - * function being called with that mutex held. - * - * Concurrency is not an issue for the moment since we don't share any - * state with KMS, so we can ignore the lock for now, but we need to - * keep it in mind if we were to change that assumption. - */ - if (!drm_dev_enter(dev, &idx)) return -ENODEV; @@ -2848,6 +2824,8 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, return -ENOMEM; } + mutex_lock(&vc4_hdmi->mutex); + spin_lock_irqsave(&vc4_hdmi->hw_lock, flags); for (i = 0; i < msg->len; i += 4) @@ -2867,7 +2845,7 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts, HDMI_WRITE(HDMI_CEC_CNTRL_1, val); spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags); - + mutex_unlock(&vc4_hdmi->mutex); drm_dev_exit(idx); return 0; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 79495ef79f09..db823efb2563 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -195,15 +195,7 @@ struct vc4_hdmi { /** * @mutex: Mutex protecting the driver access across multiple - * frameworks (KMS, ALSA). - * - * NOTE: While supported, CEC has been left out since - * cec_s_phys_addr_from_edid() might call .adap_enable and lead to a - * reentrancy issue between .get_modes (or .detect) and .adap_enable. - * Since we don't share any state between the CEC hooks and KMS', it's - * not a big deal. The only trouble might come from updating the CEC - * clock divider which might be affected by a modeset, but CEC should - * be resilient to that. + * frameworks (KMS, ALSA, CEC). */ struct mutex mutex; -- cgit v1.2.3 From c0895f80272cd4100a26129f4fb91a85bf6663a1 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:27 +0200 Subject: drm/vc4: hdmi: Simplify the hotplug handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our detect callback has a bunch of operations to perform depending on the current and last status of the connector, such a setting the CEC physical address or enabling the scrambling again. This is currently dealt with a bunch of if / else statetements that make it fairly difficult to read and extend. Let's move all that logic to a function of its own. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-5-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 63 +++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 22 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 308f7440eb6a..93df3a8e6ed9 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -273,17 +273,50 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {} static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder); +static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi, + enum drm_connector_status status) +{ + struct drm_connector *connector = &vc4_hdmi->connector; + struct edid *edid; + + /* + * NOTE: This function should really be called with + * vc4_hdmi->mutex held, but doing so results in reentrancy + * issues since cec_s_phys_addr_from_edid might call + * .adap_enable, which leads to that funtion being called with + * our mutex held. + * + * Concurrency isn't an issue at the moment since we don't share + * any state with any of the other frameworks so we can ignore + * the lock for now. + */ + + if (status == connector_status_disconnected) { + cec_phys_addr_invalidate(vc4_hdmi->cec_adap); + return; + } + + edid = drm_get_edid(connector, vc4_hdmi->ddc); + if (!edid) + return; + + cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); + kfree(edid); + + vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base); +} + static enum drm_connector_status vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) { struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); - bool connected = false; + enum drm_connector_status status = connector_status_disconnected; /* * NOTE: This function should really take vc4_hdmi->mutex, but * doing so results in reentrancy issues since - * cec_s_phys_addr_from_edid might call .adap_enable, which - * leads to that funtion being called with our mutex held. + * vc4_hdmi_handle_hotplug() can call into other functions that + * would take the mutex while it's held here. * * Concurrency isn't an issue at the moment since we don't share * any state with any of the other frameworks so we can ignore @@ -294,31 +327,17 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) if (vc4_hdmi->hpd_gpio) { if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) - connected = true; + status = connector_status_connected; } else { if (vc4_hdmi->variant->hp_detect && vc4_hdmi->variant->hp_detect(vc4_hdmi)) - connected = true; - } - - if (connected) { - if (connector->status != connector_status_connected) { - struct edid *edid = drm_get_edid(connector, vc4_hdmi->ddc); - - if (edid) { - cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); - kfree(edid); - } - } - - vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base); - pm_runtime_put(&vc4_hdmi->pdev->dev); - return connector_status_connected; + status = connector_status_connected; } - cec_phys_addr_invalidate(vc4_hdmi->cec_adap); + vc4_hdmi_handle_hotplug(vc4_hdmi, status); pm_runtime_put(&vc4_hdmi->pdev->dev); - return connector_status_disconnected; + + return status; } static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) -- cgit v1.2.3 From 165ba1aad164b7d5d5bc327fa511f6ef693b207b Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:28 +0200 Subject: drm/vc4: hdmi: Switch to detect_ctx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We'll need the locking context in future patch, so let's convert .detect to .detect_ctx. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-6-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 93df3a8e6ed9..2f0d3438b095 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -306,8 +306,9 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi, vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base); } -static enum drm_connector_status -vc4_hdmi_connector_detect(struct drm_connector *connector, bool force) +static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx, + bool force) { struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); enum drm_connector_status status = connector_status_disconnected; @@ -449,7 +450,6 @@ vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) } static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { - .detect = vc4_hdmi_connector_detect, .fill_modes = drm_helper_probe_single_connector_modes, .reset = vc4_hdmi_connector_reset, .atomic_duplicate_state = vc4_hdmi_connector_duplicate_state, @@ -457,6 +457,7 @@ static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { }; static const struct drm_connector_helper_funcs vc4_hdmi_connector_helper_funcs = { + .detect_ctx = vc4_hdmi_connector_detect_ctx, .get_modes = vc4_hdmi_connector_get_modes, .atomic_check = vc4_hdmi_connector_atomic_check, }; -- cgit v1.2.3 From d0133e7da26c5548f7e376149a6a225b4274dcb1 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:29 +0200 Subject: drm/vc4: hdmi: Move vc4_hdmi_supports_scrambling() around MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We'll need it earlier in the driver, so let's move it next to the other scrambling-related helpers. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-7-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 2f0d3438b095..cc9c60d4facf 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -124,6 +124,23 @@ static unsigned long long vc4_hdmi_encoder_compute_mode_clock(const struct drm_display_mode *mode, unsigned int bpc, enum vc4_hdmi_output_format fmt); +static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder) +{ + struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); + struct drm_display_info *display = &vc4_hdmi->connector.display_info; + + lockdep_assert_held(&vc4_hdmi->mutex); + + if (!display->is_hdmi) + return false; + + if (!display->hdmi.scdc.supported || + !display->hdmi.scdc.scrambling.supported) + return false; + + return true; +} + static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode, unsigned int bpc, enum vc4_hdmi_output_format fmt) @@ -739,23 +756,6 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) vc4_hdmi_set_hdr_infoframe(encoder); } -static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder) -{ - struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); - struct drm_display_info *display = &vc4_hdmi->connector.display_info; - - lockdep_assert_held(&vc4_hdmi->mutex); - - if (!display->is_hdmi) - return false; - - if (!display->hdmi.scdc.supported || - !display->hdmi.scdc.scrambling.supported) - return false; - - return true; -} - #define SCRAMBLING_POLLING_DELAY_MS 1000 static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) -- cgit v1.2.3 From 6bed2ea3cb3856edf37cca20753e689ee8774793 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Mon, 29 Aug 2022 15:47:30 +0200 Subject: drm/vc4: hdmi: Reset link on hotplug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During a hotplug cycle (such as a TV going out of suspend, or when the cable is disconnected and reconnected), the expectation is that the same state used before the disconnection is reused until the next commit. However, the HDMI scrambling requires that some flags are set in the monitor, and those flags are very likely to be reset when the cable has been disconnected. This will thus result in a blank display, even if the display pipeline configuration hasn't been modified or is in the exact same state. The solution we've had so far is to enable the scrambling-related bits again on reconnection, but the HDMI 2.0 specification (Section 6.1.3.1 - Scrambling Control) requires that the scrambling enable bit is set before sending any scrambled video signal. Using that solution thus breaks that expectation. The solution used by i915 is to do a full modeset on the connector so that we disable the video signal, enable the scrambling bit, and enable the video signal again. As such, we took that code and plugged it into vc4. It probably could have been turned into an helper, but it proved to be difficult for several reasons: * i915 has fairly different structures than simpler KMS drivers such as vc4, so doing some code that works with both proved to be difficult; * Other simpler drivers could reuse some of it (tegra, dw-hdmi), but it would still require to move some parameters currently stored in private structure that are needed to compute whether the scrambling is needed or not, and then inform the driver that it needs to be enabled. Some of those parameters are already in core structures (drm_display_mode, drm_display_info, bpc), but the output format isnt't. Adding it is fairly challenging since unlike the TMDS char rate or mode, there's no consensus on what format to pick in drivers, so it's not possible to write some generic code that can depend on it. For these reasons, we chose to duplicate the code for now, until someone else really needs it as well, in which case we will be able to convert it into a generic helper. Reviewed-by: Ville Syrjälä Signed-off-by: Maxime Ripard Link: https://lore.kernel.org/r/20220829134731.213478-8-maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_hdmi.c | 104 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/vc4/vc4_hdmi.c') diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index cc9c60d4facf..b2d0a3c89fbb 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -288,9 +288,103 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {} #endif -static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder); +static int reset_pipe(struct drm_crtc *crtc, + struct drm_modeset_acquire_ctx *ctx) +{ + struct drm_atomic_state *state; + struct drm_crtc_state *crtc_state; + int ret; + + state = drm_atomic_state_alloc(crtc->dev); + if (!state) + return -ENOMEM; + + state->acquire_ctx = ctx; + + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + goto out; + } + + crtc_state->connectors_changed = true; + + ret = drm_atomic_commit(state); +out: + drm_atomic_state_put(state); + + return ret; +} + +static int vc4_hdmi_reset_link(struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx) +{ + struct drm_device *drm = connector->dev; + struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector); + struct drm_encoder *encoder = &vc4_hdmi->encoder.base; + struct drm_connector_state *conn_state; + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + bool scrambling_needed; + u8 config; + int ret; + + if (!connector) + return 0; + + ret = drm_modeset_lock(&drm->mode_config.connection_mutex, ctx); + if (ret) + return ret; + + conn_state = connector->state; + crtc = conn_state->crtc; + if (!crtc) + return 0; + + ret = drm_modeset_lock(&crtc->mutex, ctx); + if (ret) + return ret; + + crtc_state = crtc->state; + if (!crtc_state->active) + return 0; + + if (!vc4_hdmi_supports_scrambling(encoder)) + return 0; + + scrambling_needed = vc4_hdmi_mode_needs_scrambling(&vc4_hdmi->saved_adjusted_mode, + vc4_hdmi->output_bpc, + vc4_hdmi->output_format); + if (!scrambling_needed) + return 0; + + if (conn_state->commit && + !try_wait_for_completion(&conn_state->commit->hw_done)) + return 0; + + ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config); + if (ret < 0) { + drm_err(drm, "Failed to read TMDS config: %d\n", ret); + return 0; + } + + if (!!(config & SCDC_SCRAMBLING_ENABLE) == scrambling_needed) + return 0; + + /* + * HDMI 2.0 says that one should not send scrambled data + * prior to configuring the sink scrambling, and that + * TMDS clock/data transmission should be suspended when + * changing the TMDS clock rate in the sink. So let's + * just do a full modeset here, even though some sinks + * would be perfectly happy if were to just reconfigure + * the SCDC settings on the fly. + */ + return reset_pipe(crtc, ctx); +} static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi, + struct drm_modeset_acquire_ctx *ctx, enum drm_connector_status status) { struct drm_connector *connector = &vc4_hdmi->connector; @@ -303,6 +397,10 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi, * .adap_enable, which leads to that funtion being called with * our mutex held. * + * A similar situation occurs with + * drm_atomic_helper_connector_hdmi_reset_link() that will call + * into our KMS hooks if the scrambling was enabled. + * * Concurrency isn't an issue at the moment since we don't share * any state with any of the other frameworks so we can ignore * the lock for now. @@ -320,7 +418,7 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi, cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap, edid); kfree(edid); - vc4_hdmi_enable_scrambling(&vc4_hdmi->encoder.base.base); + vc4_hdmi_reset_link(connector, ctx); } static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector, @@ -352,7 +450,7 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector, status = connector_status_connected; } - vc4_hdmi_handle_hotplug(vc4_hdmi, status); + vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status); pm_runtime_put(&vc4_hdmi->pdev->dev); return status; -- cgit v1.2.3