dri-devel May 2022

dri-devel@lists.freedesktop.org

344 participants
736 discussions

[PATCH] drm/gma500: Clean up some inconsistent indenting
by Jiapeng Chong 02 Jun '22

02 Jun '22

Eliminate the follow smatch warning: drivers/gpu/drm/gma500/cdv_device.c:250 cdv_errata() warn: inconsistent indenting. Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong(a)linux.alibaba.com> --- drivers/gpu/drm/gma500/cdv_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/cdv_device.c b/drivers/gpu/drm/gma500/cdv_device.c index dd32b484dd82..09684c42cbf6 100644 --- a/drivers/gpu/drm/gma500/cdv_device.c +++ b/drivers/gpu/drm/gma500/cdv_device.c @@ -247,7 +247,7 @@ static void cdv_errata(struct drm_device *dev) * Bonus Launch to work around the issue, by degrading * performance. */ - CDV_MSG_WRITE32(pci_domain_nr(pdev->bus), 3, 0x30, 0x08027108); + CDV_MSG_WRITE32(pci_domain_nr(pdev->bus), 3, 0x30, 0x08027108); } /** -- 2.20.1.7.g153144c

2 1

[PATCH v2 0/3] MediaTek MT8195 display binding
by Rex-BC Chen 02 Jun '22

02 Jun '22

Add this series to present MediaTek display binding for MT8195. The reason I send this series is Jason and Nancy's binding patches are never received by devicetree mail server. Therefore, I help them to resend binding patches. Changes for v2: 1. This patch is based on linux next-20220506. 2. Jason's patches are accepted and I drop them. [1]: https://lore.kernel.org/all/20220504091440.2052-2-nancy.lin@mediatek.com/ Nancy.Lin (3): dt-bindings: mediatek: add vdosys1 RDMA definition for mt8195 dt-bindings: reset: mt8195: add vdosys1 reset control bit dt-bindings: mediatek: add ethdr definition for mt8195 .../display/mediatek/mediatek,ethdr.yaml | 191 ++++++++++++++++++ .../display/mediatek/mediatek,mdp-rdma.yaml | 94 +++++++++ include/dt-bindings/reset/mt8195-resets.h | 45 +++++ 3 files changed, 330 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,mdp-rdma.yaml -- 2.18.0

6 22

[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

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

[PATCH v4 0/6] i915: SSEU handling updates
by Matt Roper 01 Jun '22

01 Jun '22

This series reworks i915's internal handling of slice/subslice/EU (SSEU) data to represent platforms like Xe_HP in a more natural manner and to prepare for future platforms where the masks will need to grow in size. One key idea of this series is that although we have a fixed ABI to convey SSEU data to userspace (i.e., multiple u8[] arrays with data stored at different strides), we don't need to use this cumbersome format for the driver's own internal storage. As long as we can convert into the uapi form properly when responding to the I915_QUERY ioctl, it's preferable to use an internal storage format that's easier for the driver to work with. Another key point here is that we're reaching the point where subslice (DSS) masks will soon not fit within simple u32/u64 integer values. Xe_HP SDV and DG2 platforms today have subslice (DSS) masks that are 32 bits, which maxes out the current storage of a u32. With PVC the masks are represented by a pair of 32-bit registers, requiring a bump up to at least 64-bits of storage internally. We could switch to u64 for that in the short term, but since we already know that upcoming architectures intend to provide DSS fuse bits via three or more registers it's best to switch to a representation that's more future-proof but still easy to work with in the driver code. To accomodate this, we start storing our subslice mask for Xe_HP and beyond in a new typedef that can be processed by the linux/bitmap.h operations. Finally, since no userspace for Xe_HP or beyond is using the legacy I915_GETPARAM ioctl lookups for I915_PARAM_SLICE_MASK and I915_PARAM_SUBSLICE_MASK (since they've migrated to the more flexible I915_QUERY ioctl that can return more than a simple u32 value), we take the opportunity to officially drop support for those GETPARAM lookups on modern platforms. Maintaining support for these GETPARAM lookups don't make sense for a number of reasons: * Traditional slices no longer exist, and newer ideas like gslices, cslices, mslices, etc. aren't something userspace needs to query since it can be inferred from other information. * The GETPARAM ioctl doesn't have a way to distinguish between geometry subslice masks and compute subslice masks, which are distinct on Xe_HP and beyond. * The I915_GETPARAM ioctl is limited to returning a 32-bit value, so when subslice masks begin to exceed 32-bits (on PVC), it simply can't return the entire mask. * The GETPARAM ioctl doesn't have a way to give sensible information for multi-tile devices. v2: - Switch to union of hsw/xehp formats to keep the representation in a natural format for different types of hardware. - Avoid accessing internals of intel_sseu_ss_mask_t directly outside of intel_sseu.[ch]. - Include PVC SSEU which needs the larger SS mask storage enabled by this series. v3: - Correct a BIT(s) typo that should have been BIT(ss), causing incorrect handling on gen9 platforms. v4: - Eliminate sseu->{ss,eu}_stride fields and just calculate the proper value in the UAPI code that needs them. - Handle unwanted ~u8 sign extension at the callsite instead of inside sseu_set_eus. - Use BITMAP_BITS() macro rather than passing I915_MAX_SS_FUSE_BITS around directly to bitmap operations. - Improved debugfs / dmesg reporting for Xe_HP dumps - Various assertion check improvements. Cc: Tvrtko Ursulin <tvrtko.ursulin(a)linux.intel.com> Matt Roper (6): drm/i915/xehp: Use separate sseu init function drm/i915/xehp: Drop GETPARAM lookups of I915_PARAM_[SUB]SLICE_MASK drm/i915/sseu: Simplify gen11+ SSEU handling drm/i915/sseu: Don't try to store EU mask internally in UAPI format drm/i915/sseu: Disassociate internal subslice mask representation from uapi drm/i915/pvc: Add SSEU changes drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 +- drivers/gpu/drm/i915/gt/intel_gt.c | 12 +- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 1 + drivers/gpu/drm/i915/gt/intel_sseu.c | 450 ++++++++++++------- drivers/gpu/drm/i915/gt/intel_sseu.h | 94 ++-- drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 30 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 24 +- drivers/gpu/drm/i915/i915_drv.h | 2 + drivers/gpu/drm/i915/i915_getparam.c | 11 +- drivers/gpu/drm/i915/i915_pci.c | 3 +- drivers/gpu/drm/i915/i915_query.c | 26 +- drivers/gpu/drm/i915/intel_device_info.h | 1 + 13 files changed, 398 insertions(+), 265 deletions(-) -- 2.35.3

3 11

[PATCH v3] drm/probe-helper: Make 640x480 first if no EDID
by Douglas Anderson 01 Jun '22

01 Jun '22

If we're unable to read the EDID for a display because it's corrupt / bogus / invalid then we'll add a set of standard modes for the display. Since we have no true information about the connected display, these modes are essentially guesses but better than nothing. None of the modes returned is marked as preferred, but the modes are currently sorted such that the higher resolution modes are listed first. When userspace sees these modes presented by the kernel it needs to figure out which one to pick. At least one userspace, ChromeOS [1] seems to use the rules: 1. Try to pick the first mode marked as preferred. 2. If no modes were marked as preferred then pick the first mode. The rules above seem pretty reasonable, but they have unfortunate side effect that when we have no EDID present we'll default to the highest resolution (least likely to work) mode. Let's change things slightly. In the case of a failed EDID read we still won't mark anything preferred but we _won't_ sort the modes at the end of drm_helper_probe_single_connector_modes(). The drm_add_modes_noedid() adds 640x480 first and so by skipping the call to drm_mode_sort() it will stay first. That will be a hint to userspace to default to 640x480. This change makes userspace that behaves like ChromeOS does compliant with section 4.2.2.6 (EDID Corruption Detection) of the DP 1.4a Link CTS. That section indicates that, at least on DP, if we have a corrupt EDID userspace may allow other modes to be tried but should default to 640x480 in the absence of more information. Note that if drm_add_modes_noedid() ever changes to _not_ list 640x480 first we might need to do more here, but that seems unlikely and, in any case, it would be caught by a future run of DP compliance testing. Note: this change could pave the way to further improvement to drm_helper_probe_single_connector_modes(). Specifically, in the case of no EDID we could add additional "standard" modes that are riskier than 1024x768 (the current max we add). Now that we're giving userspace the hint that it should default to 640x480 perhaps it would be OK to offer the options of the higher resolution modes just in case they work. This further improvement is left as an exercise to the reader. [1] https://source.chromium.org/chromium/chromium/src/+/a051f741d0a15caff225130… Signed-off-by: Douglas Anderson <dianders(a)chromium.org> --- Note that this is the second of two related and similar-sounding but different patches. See also ("drm/probe-helper: For DP, add 640x480 if all other modes are bad") [2]. I'm hoping to land _both_ of the patches since they address different issues. This patch addresses the case of a corrupt EDID and having 640x480 be the default in the "guessed" modes. The other patch handles the case where the EDID _isn't_ corrupt but all the modes listed can't be made with the existing situations. The two patches can land in either order. Also note that I didn't carry any Tested-by / Reviewed-by tags since the patch is now quite different (yet again for v2 to v3). [2] https://lore.kernel.org/r/20220510131309.v2.2.I4ac7f55aa446699f8c200a23c104… Changes in v3: - Don't set preferred, just disable the sort. Changes in v2: - Don't modify drm_add_modes_noedid() 'cause that'll break others - Set 640x480 as preferred in drm_helper_probe_single_connector_modes() drivers/gpu/drm/drm_probe_helper.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 682359512996..21dd60f30cc7 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -425,6 +425,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, bool verbose_prune = true; enum drm_connector_status old_status; struct drm_modeset_acquire_ctx ctx; + bool sort_list = true; WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); @@ -516,8 +517,16 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, count = drm_add_override_edid_modes(connector); if (count == 0 && (connector->status == connector_status_connected || - connector->status == connector_status_unknown)) + connector->status == connector_status_unknown)) { count = drm_add_modes_noedid(connector, 1024, 768); + /* + * Want lower res modes, like 640x480, first. That indicates + * to userspace that these are "better" modes. Since we have + * no EDID the modes are a guess anyway, so guess the safer + * mode first. + */ + sort_list = false; + } count += drm_helper_probe_add_cmdline_mode(connector); if (count == 0) goto prune; @@ -576,7 +585,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, if (list_empty(&connector->modes)) return 0; - drm_mode_sort(&connector->modes); + if (sort_list) + drm_mode_sort(&connector->modes); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] probed modes :\n", connector->base.id, connector->name); -- 2.36.0.550.gb090851708-goog

5 8

← Newer
1
...
9
10
11
12
13
14
15
...
74
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel May 2022