dri-devel June 2022

dri-devel@lists.freedesktop.org

332 participants
697 discussions

[PATCH v3] drm/v3d/v3d_drv: Fix PM disable depth imbalance
by Miaoqian Lin 02 Jun '22

02 Jun '22

The pm_runtime_enable will increase power disable depth. If the probe fails, we should use pm_runtime_disable() to balance pm_runtime_enable(). Also call disable function in remove function. Fixes: 57692c94dcbe ("drm/v3d: Introduce a new DRM driver for Broadcom V3D V3.x+") Signed-off-by: Miaoqian Lin <linmq006(a)gmail.com> --- Changes in v3: - call pm_runtime_disable() in v3d_platform_drm_remove - update commit message Changes in v2 - put pm_runtime_disable before dma_free_wc - rename dma_free to pm_disable v1: https://lore.kernel.org/r/20220105120442.14418-1-linmq006@gmail.com v2: https://lore.kernel.org/r/20220106124657.32737-1-linmq006@gmail.com --- drivers/gpu/drm/v3d/v3d_drv.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c index 1afcd54fbbd5..f3380399fe17 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.c +++ b/drivers/gpu/drm/v3d/v3d_drv.c @@ -286,7 +286,7 @@ static int v3d_platform_drm_probe(struct platform_device *pdev) ret = v3d_gem_init(drm); if (ret) - goto dma_free; + goto pm_disable; ret = v3d_irq_init(v3d); if (ret) @@ -302,7 +302,8 @@ static int v3d_platform_drm_probe(struct platform_device *pdev) v3d_irq_disable(v3d); gem_destroy: v3d_gem_destroy(drm); -dma_free: +pm_disable: + pm_runtime_disable(dev); dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr); return ret; } @@ -316,6 +317,7 @@ static int v3d_platform_drm_remove(struct platform_device *pdev) v3d_gem_destroy(drm); + pm_runtime_disable(&pdev->dev); dma_free_wc(v3d->drm.dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr); -- 2.25.1

3 4

[PATCH v1 0/5] drm/msm: clean up the hw catalog init
by Dmitry Baryshkov 02 Jun '22

02 Jun '22

Replace superfluous cfg_init functions, which just assign a static config to the struct dpu_mdss_cfg, with static instances of struct dpu_mdss_cfg. Dmitry Baryshkov (5): drm/msm/dpu: use feature bit for LM combined alpha check drm/msm/dpu: move VBIF_XINL_QOS_LVL_REMAP size to hw_catalog drm/msm/dpu: remove hwversion field from data structures drm/msm/dpu: constify struct dpu_mdss_cfg drm/msm/dpu: make dpu hardware catalog static const drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 4 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 500 ++++++++---------- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 16 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 5 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 1 - .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 5 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 7 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_merge3d.c | 1 - .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 5 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 2 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_vbif.c | 5 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 +- 27 files changed, 258 insertions(+), 334 deletions(-) -- 2.35.1

2 6

[GIT PULL] drm/msm: drm-msm-fixes-2022-06-01
by Abhinav Kumar 02 Jun '22

02 Jun '22

Hi Rob Here is the second pull request for the fixes for 5.19. This goes on top of the previous pull request. Mainly it has the foll fixes: - Fix to add minimum ICC vote in the msm_mdss pm_resume path to address bootup splats - Fix to avoid dereferencing without checking in WB encoder - Fix to avoid crash during suspend in DP driver by ensuring interrupt mask bits are updated - Remove unused code from dpu_encoder_virt_atomic_check() - Fix to remove redundant init of dsc variable Thanks Abhinav The following changes since commit 1a5409facf9ceba401f83925c6d15bae2121d29c: Merge tag 'msm-next-5.19-fixes' of https://gitlab.freedesktop.org/abhinavk/msm into msm-fixes-staging (2022-05-19 11:21:17 -0700) are available in the git repository at: https://gitlab.freedesktop.org/abhinavk/msm.git tags/msm-next-5.19-fixes-06-01 for you to fetch changes up to b9364eed9232f3d2a846f68c2307eb25c93cc2d0: drm/msm/dpu: Move min BW request and full BW disable back to mdss (2022-06-01 16:16:19 -0700) ---------------------------------------------------------------- 5.19 fixes for msm-next ---------------------------------------------------------------- Douglas Anderson (1): drm/msm/dpu: Move min BW request and full BW disable back to mdss Haowen Bai (1): drm/msm/dpu: Fix pointer dereferenced before checking Jiapeng Chong (1): drm/msm/dpu: Remove unused code Kuogee Hsieh (1): drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl() Vinod Koul (1): drm/msm/disp/dpu1: remove superfluous init drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 -------- drivers/gpu/drm/msm/dp/dp_ctrl.c | 9 +++++++-- drivers/gpu/drm/msm/msm_mdss.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 15 deletions(-)

1 0

[PATCH v2] drm/msm/dpu: Move min BW request and full BW disable back to mdss
by Douglas Anderson 01 Jun '22

01 Jun '22

In commit a670ff578f1f ("drm/msm/dpu: always use mdp device to scale bandwidth") we fully moved interconnect stuff to the DPU driver. This had no change for sc7180 but _did_ have an impact for other SoCs. It made them match the sc7180 scheme. Unfortunately, the sc7180 scheme seems like it was a bit broken. Specifically the interconnect needs to be on for more than just the DPU driver's AXI bus. In the very least it also needs to be on for the DSI driver's AXI bus. This can be seen fairly easily by doing this on a ChromeOS sc7180-trogdor class device: set_power_policy --ac_screen_dim_delay=5 --ac_screen_off_delay=10 sleep 10 cd /sys/bus/platform/devices/ae94000.dsi/power echo on > control When you do that, you'll get a warning splat in the logs about "gcc_disp_hf_axi_clk status stuck at 'off'". One could argue that perhaps what I have done above is "illegal" and that it can't happen naturally in the system because in normal system usage the DPU is pretty much always on when DSI is on. That being said: * In official ChromeOS builds (admittedly a 5.4 kernel with backports) we have seen that splat at bootup. * Even though we don't use "autosuspend" for these components, we don't use the "put_sync" variants. Thus plausibly the DSI could stay "runtime enabled" past when the DPU is enabled. Techncially we shouldn't do that if the DPU's suspend ends up yanking our clock. Let's change things such that the "bare minimum" request for the interconnect happens in the mdss driver again. That means that all of the children can assume that the interconnect is on at the minimum bandwidth. We'll then let the DPU request the higher amount that it wants. It should be noted that this isn't as hacky of a solution as it might initially appear. Specifically: * Since MDSS and DPU individually get their own references to the interconnect then the framework will actually handle aggregating them. The two drivers are _not_ clobbering each other. * When the Qualcomm interconnect driver aggregates it takes the max of all the peaks. Thus having MDSS request a peak, as we're doing here, won't actually change the total interconnect bandwidth (it won't be added to the request for the DPU). This perhaps explains why the "average" requested in MDSS was historically 0 since that one _would_ be added in. NOTE also that in the downstream ChromeOS 5.4 and 5.15 kernels, we're also seeing some RPMH hangs that are addressed by this fix. These hangs are showing up in the field and on _some_ devices with enough stress testing of suspend/resume. Specifically right at suspend time with a stack crawl that looks like this (from chromeos-5.15 tree): rpmh_write_batch+0x19c/0x240 qcom_icc_bcm_voter_commit+0x210/0x420 qcom_icc_set+0x28/0x38 apply_constraints+0x70/0xa4 icc_set_bw+0x150/0x24c dpu_runtime_resume+0x50/0x1c4 pm_generic_runtime_resume+0x30/0x44 __genpd_runtime_resume+0x68/0x7c genpd_runtime_resume+0x12c/0x20c __rpm_callback+0x98/0x138 rpm_callback+0x30/0x88 rpm_resume+0x370/0x4a0 __pm_runtime_resume+0x80/0xb0 dpu_kms_enable_commit+0x24/0x30 msm_atomic_commit_tail+0x12c/0x630 commit_tail+0xac/0x150 drm_atomic_helper_commit+0x114/0x11c drm_atomic_commit+0x68/0x78 drm_atomic_helper_disable_all+0x158/0x1c8 drm_atomic_helper_suspend+0xc0/0x1c0 drm_mode_config_helper_suspend+0x2c/0x60 msm_pm_prepare+0x2c/0x40 pm_generic_prepare+0x30/0x44 genpd_prepare+0x80/0xd0 device_prepare+0x78/0x17c dpm_prepare+0xb0/0x384 dpm_suspend_start+0x34/0xc0 We don't completely understand all the mechanisms in play, but the hang seemed to come and go with random factors. It's not terribly surprising that the hang is gone after this patch since the line of code that was failing is no longer present in the kernel. Fixes: a670ff578f1f ("drm/msm/dpu: always use mdp device to scale bandwidth") Fixes: c33b7c0389e1 ("drm/msm/dpu: add support for clk and bw scaling for display") Signed-off-by: Douglas Anderson <dianders(a)chromium.org> --- Changes in v2: - Don't set bandwidth in init. drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 ---- drivers/gpu/drm/msm/msm_mdss.c | 57 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 2b9d931474e0..3025184053e0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -49,8 +49,6 @@ #define DPU_DEBUGFS_DIR "msm_dpu" #define DPU_DEBUGFS_HWMASKNAME "hw_log_mask" -#define MIN_IB_BW 400000000ULL /* Min ib vote 400MB */ - static int dpu_kms_hw_init(struct msm_kms *kms); static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms); @@ -1303,15 +1301,9 @@ static int __maybe_unused dpu_runtime_resume(struct device *dev) struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *encoder; struct drm_device *ddev; - int i; ddev = dpu_kms->dev; - WARN_ON(!(dpu_kms->num_paths)); - /* Min vote of BW is required before turning on AXI clk */ - for (i = 0; i < dpu_kms->num_paths; i++) - icc_set_bw(dpu_kms->path[i], 0, Bps_to_icc(MIN_IB_BW)); - rc = clk_bulk_prepare_enable(dpu_kms->num_clocks, dpu_kms->clocks); if (rc) { DPU_ERROR("clock enable failed rc:%d\n", rc); diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index 0454a571adf7..e13c5c12b775 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -5,6 +5,7 @@ #include <linux/clk.h> #include <linux/delay.h> +#include <linux/interconnect.h> #include <linux/irq.h> #include <linux/irqchip.h> #include <linux/irqdesc.h> @@ -25,6 +26,8 @@ #define UBWC_CTRL_2 0x150 #define UBWC_PREDICTION_MODE 0x154 +#define MIN_IB_BW 400000000UL /* Min ib vote 400MB */ + struct msm_mdss { struct device *dev; @@ -36,8 +39,47 @@ struct msm_mdss { unsigned long enabled_mask; struct irq_domain *domain; } irq_controller; + struct icc_path *path[2]; + u32 num_paths; }; +static int msm_mdss_parse_data_bus_icc_path(struct device *dev, + struct msm_mdss *msm_mdss) +{ + struct icc_path *path0 = of_icc_get(dev, "mdp0-mem"); + struct icc_path *path1 = of_icc_get(dev, "mdp1-mem"); + + if (IS_ERR_OR_NULL(path0)) + return PTR_ERR_OR_ZERO(path0); + + msm_mdss->path[0] = path0; + msm_mdss->num_paths = 1; + + if (!IS_ERR_OR_NULL(path1)) { + msm_mdss->path[1] = path1; + msm_mdss->num_paths++; + } + + return 0; +} + +static void msm_mdss_put_icc_path(void *data) +{ + struct msm_mdss *msm_mdss = data; + int i; + + for (i = 0; i < msm_mdss->num_paths; i++) + icc_put(msm_mdss->path[i]); +} + +static void msm_mdss_icc_request_bw(struct msm_mdss *msm_mdss, unsigned long bw) +{ + int i; + + for (i = 0; i < msm_mdss->num_paths; i++) + icc_set_bw(msm_mdss->path[i], 0, Bps_to_icc(bw)); +} + static void msm_mdss_irq(struct irq_desc *desc) { struct msm_mdss *msm_mdss = irq_desc_get_handler_data(desc); @@ -136,6 +178,13 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss) { int ret; + /* + * Several components have AXI clocks that can only be turned on if + * the interconnect is enabled (non-zero bandwidth). Let's make sure + * that the interconnects are at least at a minimum amount. + */ + msm_mdss_icc_request_bw(msm_mdss, MIN_IB_BW); + ret = clk_bulk_prepare_enable(msm_mdss->num_clocks, msm_mdss->clocks); if (ret) { dev_err(msm_mdss->dev, "clock enable failed, ret:%d\n", ret); @@ -178,6 +227,7 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss) static int msm_mdss_disable(struct msm_mdss *msm_mdss) { clk_bulk_disable_unprepare(msm_mdss->num_clocks, msm_mdss->clocks); + msm_mdss_icc_request_bw(msm_mdss, 0); return 0; } @@ -271,6 +321,13 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5 dev_dbg(&pdev->dev, "mapped mdss address space @%pK\n", msm_mdss->mmio); + ret = msm_mdss_parse_data_bus_icc_path(&pdev->dev, msm_mdss); + if (ret) + return ERR_PTR(ret); + ret = devm_add_action_or_reset(&pdev->dev, msm_mdss_put_icc_path, msm_mdss); + if (ret) + return ERR_PTR(ret); + if (is_mdp5) ret = mdp5_mdss_parse_clock(pdev, &msm_mdss->clocks); else -- 2.36.1.255.ge46751e96f-goog

5 8

[PATCH] drm/ssd130x: Only define a SPI device ID table when built as a module
by Javier Martinez Canillas 01 Jun '22

01 Jun '22

The kernel test robot reports a compile warning due the ssd130x_spi_table variable being defined but not used. This happen when ssd130x-spi driver is built-in instead of being built as a module, i.e: CC drivers/gpu/drm/solomon/ssd130x-spi.o AR drivers/base/firmware_loader/built-in.a AR drivers/base/built-in.a CC kernel/trace/trace.o drivers/gpu/drm/solomon/ssd130x-spi.c:155:35: warning: ‘ssd130x_spi_table’ defined but not used [-Wunused-const-variable=] 155 | static const struct spi_device_id ssd130x_spi_table[] = { | ^~~~~~~~~~~~~~~~~ The driver shouldn't need a SPI device ID table and only have an OF device ID table, but the former is needed to workaround an issue in the SPI core. This always reports a MODALIAS of the form "spi:<device>" even for devices registered through Device Trees. But the table is only needed when the driver built as a module to populate the .ko alias info. It's not needed when the driver is built-in the kernel. Fixes: 74373977d2ca ("drm/solomon: Add SSD130x OLED displays SPI support") Reported-by: kernel test robot <lkp(a)intel.com> Signed-off-by: Javier Martinez Canillas <javierm(a)redhat.com> --- drivers/gpu/drm/solomon/ssd130x-spi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c index 43722adab1f8..07802907e39a 100644 --- a/drivers/gpu/drm/solomon/ssd130x-spi.c +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c @@ -143,6 +143,7 @@ static const struct of_device_id ssd130x_of_match[] = { }; MODULE_DEVICE_TABLE(of, ssd130x_of_match); +#if IS_MODULE(CONFIG_DRM_SSD130X_SPI) /* * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even * if the device was registered via OF. This means that the module will not be @@ -160,6 +161,7 @@ static const struct spi_device_id ssd130x_spi_table[] = { { /* sentinel */ } }; MODULE_DEVICE_TABLE(spi, ssd130x_spi_table); +#endif static struct spi_driver ssd130x_spi_driver = { .driver = { -- 2.36.1

2 2

[PATCH v3 1/2] drm/probe-helper: Add helper for drm_helper_probe_single_connector_modes()
by Douglas Anderson 01 Jun '22

01 Jun '22

The drm_helper_probe_single_connector_modes() is a bit long. Let's break a chunk off to update and validate modes. This helps avoid one goto and also will allow us to more easily call the helper a second time in a future patch without adding looping or another goto. This change is intended to be a no-op change--just code movement. Signed-off-by: Douglas Anderson <dianders(a)chromium.org> Reviewed-by: Abhinav Kumar <quic_abhinavk(a)quicinc.com> Reviewed-by: Thomas Zimmermann <tzimmermann(a)suse.de> --- Changes in v3: - Removed WARN_ON Changes in v2: - Two underscores for __drm_helper_update_and_validate(). - Return err and use WARN_ON instead of returning a bool. drivers/gpu/drm/drm_probe_helper.c | 106 ++++++++++++++++------------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 682359512996..2570d47e7eab 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -354,6 +354,61 @@ drm_helper_probe_detect(struct drm_connector *connector, } EXPORT_SYMBOL(drm_helper_probe_detect); +static int __drm_helper_update_and_validate(struct drm_connector *connector, + uint32_t maxX, uint32_t maxY, + struct drm_modeset_acquire_ctx *ctx) +{ + struct drm_device *dev = connector->dev; + struct drm_display_mode *mode; + int mode_flags = 0; + int ret; + + drm_connector_list_update(connector); + + if (connector->interlace_allowed) + mode_flags |= DRM_MODE_FLAG_INTERLACE; + if (connector->doublescan_allowed) + mode_flags |= DRM_MODE_FLAG_DBLSCAN; + if (connector->stereo_allowed) + mode_flags |= DRM_MODE_FLAG_3D_MASK; + + list_for_each_entry(mode, &connector->modes, head) { + if (mode->status != MODE_OK) + continue; + + mode->status = drm_mode_validate_driver(dev, mode); + if (mode->status != MODE_OK) + continue; + + mode->status = drm_mode_validate_size(mode, maxX, maxY); + if (mode->status != MODE_OK) + continue; + + mode->status = drm_mode_validate_flag(mode, mode_flags); + if (mode->status != MODE_OK) + continue; + + ret = drm_mode_validate_pipeline(mode, connector, ctx, + &mode->status); + if (ret) { + drm_dbg_kms(dev, + "drm_mode_validate_pipeline failed: %d\n", + ret); + + if (drm_WARN_ON_ONCE(dev, ret != -EDEADLK)) + mode->status = MODE_ERROR; + else + return -EDEADLK; + } + + if (mode->status != MODE_OK) + continue; + mode->status = drm_mode_validate_ycbcr420(mode, connector); + } + + return 0; +} + /** * drm_helper_probe_single_connector_modes - get complete set of display modes * @connector: connector to probe @@ -421,7 +476,6 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, const struct drm_connector_helper_funcs *connector_funcs = connector->helper_private; int count = 0, ret; - int mode_flags = 0; bool verbose_prune = true; enum drm_connector_status old_status; struct drm_modeset_acquire_ctx ctx; @@ -519,52 +573,12 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, connector->status == connector_status_unknown)) count = drm_add_modes_noedid(connector, 1024, 768); count += drm_helper_probe_add_cmdline_mode(connector); - if (count == 0) - goto prune; - - drm_connector_list_update(connector); - - if (connector->interlace_allowed) - mode_flags |= DRM_MODE_FLAG_INTERLACE; - if (connector->doublescan_allowed) - mode_flags |= DRM_MODE_FLAG_DBLSCAN; - if (connector->stereo_allowed) - mode_flags |= DRM_MODE_FLAG_3D_MASK; - - list_for_each_entry(mode, &connector->modes, head) { - if (mode->status != MODE_OK) - continue; - - mode->status = drm_mode_validate_driver(dev, mode); - if (mode->status != MODE_OK) - continue; - - mode->status = drm_mode_validate_size(mode, maxX, maxY); - if (mode->status != MODE_OK) - continue; - - mode->status = drm_mode_validate_flag(mode, mode_flags); - if (mode->status != MODE_OK) - continue; - - ret = drm_mode_validate_pipeline(mode, connector, &ctx, - &mode->status); - if (ret) { - drm_dbg_kms(dev, - "drm_mode_validate_pipeline failed: %d\n", - ret); - - if (drm_WARN_ON_ONCE(dev, ret != -EDEADLK)) { - mode->status = MODE_ERROR; - } else { - drm_modeset_backoff(&ctx); - goto retry; - } + if (count != 0) { + ret = __drm_helper_update_and_validate(connector, maxX, maxY, &ctx); + if (ret == -EDEADLK) { + drm_modeset_backoff(&ctx); + goto retry; } - - if (mode->status != MODE_OK) - continue; - mode->status = drm_mode_validate_ycbcr420(mode, connector); } prune: -- 2.36.0.550.gb090851708-goog

3 4

[PATCH v2 0/3] drm/panel: simple: add bus-format support for panel-dpi
by Max Krummenacher 01 Jun '22

01 Jun '22

From: Max Krummenacher <max.krummenacher(a)toradex.com> Commit 4a1d0dbc8332 ("drm/panel: simple: add panel-dpi support") added support for defining a panel from device tree provided data. However support for setting the bus format is missing, so that with the current implementation a 'panel-dpi' panel can only be used if the driver of the display interface connected can cope with a missing bus_format. This patch series defines the new property bus-format and adds it to the panel-dpi implementation. Check initial discussions [1] and [2]. [1] https://lore.kernel.org/all/20220201110717.3585-1-cniedermaier@dh-electroni… [2] https://lore.kernel.org/all/20220222084723.14310-1-max.krummenacher@toradex… Changes in v2: - Fix errors found by dt_binding_check Max Krummenacher (3): dt-bindings: display: add new bus-format property for panel-dpi dt-bindings: display: startek,startek-kd050c: allow bus-format property drm/panel: simple: add bus-format support for panel-dpi .../bindings/display/panel/panel-dpi.yaml | 11 +++++ .../display/panel/startek,startek-kd050c.yaml | 1 + drivers/gpu/drm/panel/panel-simple.c | 43 +++++++++++++++++++ .../dt-bindings/display/dt-media-bus-format.h | 23 ++++++++++ 4 files changed, 78 insertions(+) create mode 100644 include/dt-bindings/display/dt-media-bus-format.h -- 2.20.1

2 4

[PATCH v2] drm: Don't block HDR_OUTPUT_METADATA on unknown EOTF
by Harry Wentland 01 Jun '22

01 Jun '22

The EDID of an HDR display defines EOTFs that are supported by the display and can be set in the HDR metadata infoframe. Userspace is expected to read the EDID and set an appropriate HDR_OUTPUT_METADATA. In drm_parse_hdr_metadata_block the kernel reads the supported EOTFs from the EDID and stores them in the drm_connector->hdr_sink_metadata. While doing so it also filters the EOTFs to the EOTFs the kernel knows about. When an HDR_OUTPUT_METADATA is set it then checks to make sure the EOTF is a supported EOTF. In cases where the kernel doesn't know about a new EOTF this check will fail, even if the EDID advertises support. Since it is expected that userspace reads the EDID to understand what the display supports it doesn't make sense for DRM to block an HDR_OUTPUT_METADATA if it contains an EOTF the kernel doesn't understand. This comes with the added benefit of future-proofing metadata support. If the spec defines a new EOTF there is no need to update DRM and an compositor can immediately make use of it. Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/609 v2: Distinguish EOTFs defind in kernel and ones defined in EDID in the commit description (Pekka) Signed-off-by: Harry Wentland <harry.wentland(a)amd.com> Cc: ppaalanen(a)gmail.com Cc: sebastian.wick(a)redhat.com Cc: vprosyak(a)amd.com Cc: Uma Shankar <uma.shankar(a)intel.com> Cc: Ville Syrjälä <ville.syrjala(a)linux.intel.com> Acked-by: Pekka Paalanen <pekka.paalanen(a)collabora.com> --- drivers/gpu/drm/drm_edid.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 12893e7be89b..223f96a72064 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5691,10 +5691,8 @@ drm_hdmi_infoframe_set_hdr_metadata(struct hdmi_drm_infoframe *frame, /* Sink EOTF is Bit map while infoframe is absolute values */ if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, - connector->hdr_sink_metadata.hdmi_type1.eotf)) { - DRM_DEBUG_KMS("EOTF Not Supported\n"); - return -EINVAL; - } + connector->hdr_sink_metadata.hdmi_type1.eotf)) + DRM_DEBUG_KMS("Unknown EOTF %d\n", hdr_metadata->hdmi_metadata_type1.eotf); err = hdmi_drm_infoframe_init(frame); if (err < 0) -- 2.36.1

1 0

Re: [PATCH v6 3/6] drm: Add driver for Solomon SSD130x OLED displays
by Dominik Kierner 01 Jun '22

01 Jun '22

Hello Javier, I was working on a SSD130x driver as well, although with a different (drm_panel) approach and hit a bit of a roadblock. Now that Your driver is quite a bit maturer than mine, I will happily provide You with the source of my draft, so that any useful bits can be incorporated in Your driver. I know that links are a bit frowned upon, but I'd rather avoid cluttering the thread with my draft code, which is unfinished and incompatible with the code in this thread. https://github.com/dh-electronics/panel-solomon-ssd130x-draft https://github.com/dh-electronics/panel-solomon-ssd130x-draft/tree/drm-ssd1… The code was designed as a rewrite from scratch, as I assumed that a new DRM driver that accommodates for I2C, 3- and 4-wire-SPI, will probably need a new DTS interface for differentiating the protocol-specific drivers, which would obviously break compatibility. I do have few suggestions though: # Atomic Configuration of Display Driving Regulator and the Charge Pump The regulator VBAT is the SSD1306-specific low-voltage (3.3 V to 5 V) regulator for the charge pump and takes the place of the voltage regulator VCC, that would otherwise supply the OLED driver block with 7 V to 15 V. The charge pump is never enabled when a VCC with 7 V to 15 V is present. Configuring the charge pump based on the available regulators, would provide an atomic configuration for either low-voltage + charge pump or the regular voltage. This way, the device tree boolean for enabling the charge pump could be removed by probing for an optional VBAT first, which replaces VCC, and falling back to a mandatory VCC otherwise: ``` [...] struct ssd130x_panel { ... struct regulator *vdd; /* Core logic supply */ union { struct regulator *vcc; /* Panel driving supply */ struct regulator *vbat; /* Charge pump regulator supply */ }; struct backlight_device *backlight; struct { unsigned int com_scan_dir_inv : 1; unsigned int com_seq_pin_cfg : 1; unsigned int com_lr_remap : 1; unsigned int seg_remap : 1; unsigned int inverse_display : 1; unsigned int use_charge_pump : 1; uint8_t height; uint8_t width; uint8_t height_mm; uint8_t width_mm; uint8_t display_start_line; uint8_t com_offset ; uint8_t contrast; uint8_t pre_charge_period_dclocks_phase1; uint8_t pre_charge_period_dclocks_phase2; uint8_t vcomh_deselect_level; uint8_t clock_divide_ratio; uint8_t oscillator_frequency; } display_settings; bool prepared; bool enabled; [...] ssd130x->vbat = devm_regulator_get_optional(dev, "vbat"); if (IS_ERR(ssd130x->vbat)) { ret = PTR_ERR(ssd130x->vbat); if (ret != -ENODEV) { if (ret != -EPROBE_DEFER) dev_err(dev, "failed to request regulator: %d\n", ret); return ret; } ssd130x->vbat = NULL; } else { ssd130x->display_settings.use_charge_pump = true; } /* Get panel driving supply */ If (!ssd130x->vbat) { ssd130x->vcc = devm_regulator_get(dev, "vcc"); if (IS_ERR(ssd130x->vcc)){ ret = PTR_ERR(ssd130x->vcc); return ret; } else { ssd130x->display_settings.use_charge_pump = false; } } ``` Splitting in VCC/VBAT and VDD and enforcing their presence is of course compatibility breaking. https://github.com/dh-electronics/panel-solomon-ssd130x-draft/blob/drm-ssd1… https://github.com/dh-electronics/panel-solomon-ssd130x-draft/blob/drm-ssd1… # Static or Dynamic Configuration for SPI-Modes 3-Wire and 4-Wire For the SPI-protocol drivers I see two possible approaches: * Dynamic configuration by determining the presence/absence of the D/C-GPIO and assigning the functions accordingly. This way a single driver file for both SPI modes could be sufficient. * Static configuration by using the device-tree names (ssd130x-spi-3wire/-4wire) to differentiate between the SPI protocol drivers. This would obviously necessitate two drivers files. Which one do you think would be the best approach for this? # DRM Mode Configuration via Device Tree In the old fbdev driver, the display modes are hard-coded, which means for every new display configuration, a new patch needs to be mainlined, which slows down official Kernel support and puts burden on the maintainers. Additionally, with the DRM-subsystem supporting height and length information, for scaling, this opens up a lot of new combinations. The SSD1306 for example, is available in multiple resolutions like 128x64 and 96x16 and comes in different sizes per resolution as well. Just to name a few: * 128x64 0.96" (22x11mm) * 128x64 1.3" (30x15mm) * 96x16 0.69" (18x3mm) Instead of hard-coding, I would suggest something along the lines of of_get_drm_display_mode(). The displays won't need to support multiple modes at the same time, let alone support for switching between them, so the one-time invocation of this expensive function might be worth it. maybe a new and simpler function that could be named: of_get_drm_display_mode_simple() Providing a mode could later prove useful for a conversion to drm_panel, if that is feasible. But for a function like this, I have to chicken out. # DRM Panel The reason why I decided for the drm_panel approach in my code, was power management and a clean handling of the software backlight dependency, which requires powered display regulators to be powered. Prepare/unprepare would power on/off the display logic regulator VDD. Enable/disable would power on/off VCC/VBAT, optionally turn on/off the charge pump and send the DISPLAY_ON/OFF commands. The SSD1305's PWM part would likely be placed in enable/disable as well. What is Your opinion on using drm_panel for Your driver? Mit freundlichen Grüßen / Best regards Dominik Kierner Student Employee Research & Development DH electronics

3 2

[PATCH] dma-fence: allow dma fence to have their own lock
by Sergey Senozhatsky 01 Jun '22

01 Jun '22

RFC I don't have a good name for this yet and I did not spend any time on documentataion (for that reason) We create fences (out fences) as part of operations execution, which are short-lived objects, we want to release all memory after operation execution is completed or when operation gets cancelled/deleted via ioctl(). This creates a bit of a problem. DMA fences are refcounted objects and exporter never knows when importer imports a fence or puts its refcount, so exporter never knows when fence will be destoyed, which should not be a problem for refcounted objects, but here comes the twist... operation A - creates and exports out fence X ... user-space imports fence X operation A - finishes execution, signals fence X kfree operation A, put dma_fence DMA fences are designed to borrow spinlock that DMA fences use to protect struct dma_fence members: struct dma_fence { spinlock_t *lock; const struct dma_fence_ops *ops; ..... }; void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno); So the `lock` should have at least same lifespan as the DMA fence that borrows it, which is impossible to guarantee in our case. When we kfree operation A struct we also kfree ->lock that operation lends to DMA fence, which outlives operation A (depending on what fence importers do and when they drop imported fence refcount). This patch adds a new memnber to struct dma_fence: __lock_inplace. Which is a lock that DMA fence will use to protect its own data when it cannot reliably borrow a lock from the outside object. I also had a patch that puts inplace and borrowed locks to an unnamed uninon and adds one more dma_fence_flag_bits to distinguish between fences with borrowed and inplace locks struct dma_fence { uninon { spinlock_t *lock; spinlock_t __lock_inplace; }; ... }; And then instead of locking/unlocking ->lock directly we would use dma_fence_lock_irqsave()/dma_fence_unlock_irqrestore() macros which would check fence flags and either use borrowed lock or inplace lock. But after seeing how owten drivers directly access fence ->lock I decided to scratch that approach and just add extra spinlock member. Not-Yet-Signed-off-by: Sergey Senozhatsky <senozhatsky(a)chromium.org> --- drivers/dma-buf/dma-fence.c | 10 ++++++++++ include/linux/dma-fence.h | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 066400ed8841..7ae40b8adb73 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -958,3 +958,13 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, trace_dma_fence_init(fence); } EXPORT_SYMBOL(dma_fence_init); + +void dma_fence_inplace_lock_init(struct dma_fence *fence, + const struct dma_fence_ops *ops, + u64 context, u64 seqno) +{ + spin_lock_init(&fence->__lock_inplace); + + dma_fence_init(fence, ops, &fence->__lock_inplace, context, seqno); +} +EXPORT_SYMBOL(dma_fence_inplace_lock_init); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 1ea691753bd3..6b15a0d2eccf 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -64,6 +64,8 @@ struct dma_fence_cb; */ struct dma_fence { spinlock_t *lock; + spinlock_t __lock_inplace; + const struct dma_fence_ops *ops; /* * We clear the callback list on kref_put so that by the time we @@ -262,6 +264,10 @@ struct dma_fence_ops { void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno); +void dma_fence_inplace_lock_init(struct dma_fence *fence, + const struct dma_fence_ops *ops, + u64 context, u64 seqno); + void dma_fence_release(struct kref *kref); void dma_fence_free(struct dma_fence *fence); void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq); -- 2.36.1.124.g0e6072fb45-goog

4 12

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel June 2022