Re: [PATCH 1/2] drm/msm: don't free the IRQ if it was not requested
On 5/6/2022 6:00 PM, Dmitry Baryshkov wrote:
As msm_drm_uninit() is called from the msm_drm_init() error path,
additional care should be necessary as not to call the free_irq() for
the IRQ that was not requested before (because an error occured earlier
than the request_irq() call).
This fixed the issue reported with the following backtrace:
[8.571329] Trying to free already-free IRQ 187
[8.571339] WARNING: CPU: 0 PID: 76 at kernel/irq/manage.c:1895
free_irq+0x1e0/0x35c
[8.588746] Modules linked in: pmic_glink pdr_interface fastrpc qrtr_smd
snd_soc_hdmi_codec msm fsa4480 gpu_sched drm_dp_aux_bus qrtr i2c_qcom_geni
crct10dif_ce qcom_stats qcom_q6v5_pas drm_display_helper gpi qcom_pil_info
drm_kms_helper qcom_q6v5 qcom_sysmon qcom_common qcom_glink_smem qcom_rng
mdt_loader qmi_helpers phy_qcom_qmp ufs_qcom typec qnoc_sm8350 socinfo
rmtfs_mem fuse drm ipv6
[8.624154] CPU: 0 PID: 76 Comm: kworker/u16:2 Not tainted
5.18.0-rc5-next-20220506-00033-g6cee8cab6089-dirty #419
[8.624161] Hardware name: Qualcomm Technologies, Inc. SM8350 HDK (DT)
[8.641496] Workqueue: events_unbound deferred_probe_work_func
[8.647510] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[8.654681] pc : free_irq+0x1e0/0x35c
[8.658454] lr : free_irq+0x1e0/0x35c
[8.662228] sp : 88ab3950
[8.665642] x29: 88ab3950 x28: x27: 16350f56a700
[8.672994] x26: 1635025df080 x25: 16350251badc x24: 16350251bb90
[8.680343] x23: x22: 00bb x21: 16350e8f9800
[8.687690] x20: 16350251ba00 x19: 16350cbd5880 x18:
[8.695039] x17: x16: a2dd12179434 x15: a2dd1431d02d
[8.702391] x14: x13: a2dd1431d028 x12: 662d79646165726c
[8.709740] x11: a2dd13fd2438 x10: 000a x9 : 00bb
[8.717111] x8 : a2dd13fd23f0 x7 : 88ab3750 x6 : f202
[8.724487] x5 : 16377e870a18 x4 : f202 x3 : 735a6ae1b000
[8.731851] x2 : x1 : x0 : 1635015f8000
[8.739217] Call trace:
[8.741755] free_irq+0x1e0/0x35c
[8.745198] msm_drm_uninit.isra.0+0x14c/0x294 [msm]
[8.750548] msm_drm_bind+0x28c/0x5d0 [msm]
[8.755081] try_to_bring_up_aggregate_device+0x164/0x1d0
[8.760657] __component_add+0xa0/0x170
[8.764626] component_add+0x14/0x20
[8.768337] dp_display_probe+0x2a4/0x464 [msm]
[8.773242] platform_probe+0x68/0xe0
[8.777043] really_probe.part.0+0x9c/0x28c
[8.781368] __driver_probe_device+0x98/0x144
[8.785871] driver_probe_device+0x40/0x140
[8.790191] __device_attach_driver+0xb4/0x120
[8.794788] bus_for_each_drv+0x78/0xd0
[8.798751] __device_attach+0xdc/0x184
[8.802713] device_initial_probe+0x14/0x20
[8.807031] bus_probe_device+0x9c/0xa4
[8.810991] deferred_probe_work_func+0x88/0xc0
[8.815667] process_one_work+0x1d0/0x320
[8.819809] worker_thread+0x14c/0x444
[8.823688] kthread+0x10c/0x110
[8.827036] ret_from_fork+0x10/0x20
Reported-by: Bjorn Andersson
Fixes: f026e431cf86 ("drm/msm: Convert to Linux IRQ interfaces")
Signed-off-by: Dmitry Baryshkov
I was looking at how other drm drivers handle this and they also have a
similar logic. So this is a good addition and thanks to the -EDEFER path
getting exposed we finally uncovered this.
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/msm_drv.c | 7 ++-
drivers/gpu/drm/msm/msm_kms.h | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 4a3dda23e3e0..44485363f37a 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -113,6 +113,8 @@ static int msm_irq_postinstall(struct drm_device *dev)
static int msm_irq_install(struct drm_device *dev, unsigned int irq)
{
+ struct msm_drm_private *priv = dev->dev_private;
+ struct msm_kms *kms = priv->kms;
int ret;
if (irq == IRQ_NOTCONNECTED)
@@ -124,6 +126,8 @@ static int msm_irq_install(struct drm_device *dev, unsigned
int irq)
if (ret)
return ret;
+ kms->irq_requested = true;
+
ret = msm_irq_postinstall(dev);
if (ret) {
free_irq(irq, dev);
@@ -139,7 +143,8 @@ static void msm_irq_uninstall(struct drm_device *dev)
struct msm_kms *kms = priv->kms;
kms->funcs->irq_uninstall(kms);
- free_irq(kms->irq, dev);
+ if (kms->irq_requested)
+ free_irq(kms->irq, dev);
}
struct msm_vblank_work {
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index ab25fff271f9..f8ed7588928c 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -148,6 +148,7 @@ struct msm_kms {
/* irq number to be passed on to msm_irq_install */
int irq;
+ bool
Re: [Freedreno] [PATCH] drm/msm/dpu: remove NULL-ness check in dpu_hw_intr_destroy
On 5/7/2022 4:40 AM, Dmitry Baryshkov wrote:
There is no need to check that kfree() argument is not NULL. Remove
extra check and call kfree() unconditionally.
Reported-by: kernel test robot
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index fa4f99034a08..915250b7f122 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -433,8 +433,7 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
void dpu_hw_intr_destroy(struct dpu_hw_intr *intr)
{
- if (intr)
- kfree(intr);
+ kfree(intr);
}
int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
[PATCH v2 3/3] drm/msm: stop storing the array of CRTCs in struct msm_drm_private
Handling the array of CRTC duplicate the struct msm_drm_private
duplicates a list of CRTCs in the drm_device. Drop it and use the
existing list for CRTC enumeration.
Signed-off-by: Dmitry Baryshkov
Reported-by: kernel test robot
Signed-off-by: Dmitry Baryshkov
---
Changes since v1:
- Intialize the index variable in msm_drm_init() / event thread
initialization.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 2 +-
drivers/gpu/drm/msm/msm_drv.c| 29
drivers/gpu/drm/msm/msm_drv.h| 3 +--
5 files changed, 19 insertions(+), 19 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..c84859fb2d9b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -808,7 +808,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
ret = PTR_ERR(crtc);
return ret;
}
- priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->num_crtcs++;
}
/* All CRTCs are compatible with all encoders */
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index fb48c8c19ec3..7449c1693e45 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -337,7 +337,7 @@ static int modeset_init(struct mdp4_kms *mdp4_kms)
goto fail;
}
- priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->num_crtcs++;
}
/*
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 3d5621a68f85..36808990f840 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -497,7 +497,7 @@ static int modeset_init(struct mdp5_kms *mdp5_kms)
DRM_DEV_ERROR(dev->dev, "failed to construct crtc %d
(%d)\n", i, ret);
goto fail;
}
- priv->crtcs[priv->num_crtcs++] = crtc;
+ priv->num_crtcs++;
}
/*
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 4a3dda23e3e0..db676a142ac1 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -144,7 +144,7 @@ static void msm_irq_uninstall(struct drm_device *dev)
struct msm_vblank_work {
struct work_struct work;
- int crtc_id;
+ struct drm_crtc *crtc;
bool enable;
struct msm_drm_private *priv;
};
@@ -157,15 +157,15 @@ static void vblank_ctrl_worker(struct work_struct *work)
struct msm_kms *kms = priv->kms;
if (vbl_work->enable)
- kms->funcs->enable_vblank(kms, priv->crtcs[vbl_work->crtc_id]);
+ kms->funcs->enable_vblank(kms, vbl_work->crtc);
else
- kms->funcs->disable_vblank(kms, priv->crtcs[vbl_work->crtc_id]);
+ kms->funcs->disable_vblank(kms, vbl_work->crtc);
kfree(vbl_work);
}
static int vblank_ctrl_queue_work(struct msm_drm_private *priv,
- int crtc_id, bool enable)
+ struct drm_crtc *crtc, bool enable)
{
struct msm_vblank_work *vbl_work;
@@ -175,7 +175,7 @@ static int vblank_ctrl_queue_work(struct msm_drm_private
*priv,
INIT_WORK(_work->work, vblank_ctrl_worker);
- vbl_work->crtc_id = crtc_id;
+ vbl_work->crtc = crtc;
vbl_work->enable = enable;
vbl_work->priv = priv;
@@ -349,6 +349,7 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
struct msm_drm_private *priv = dev_get_drvdata(dev);
struct drm_device *ddev;
struct msm_kms *kms;
+ struct drm_crtc *crtc;
int ret, i;
if (drm_firmware_drivers_only())
@@ -422,12 +423,14 @@ static int msm_drm_init(struct device *dev, const struct
drm_driver *drv)
ddev->mode_config.funcs = _config_funcs;
ddev->mode_config.helper_private = _config_helper_funcs;
- for (i = 0; i < priv->num_crtcs; i++) {
+ drm_for_each_crtc(crtc, ddev) {
+ i = drm_crtc_index(crtc);
+
/* initialize event thread */
- priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id;
+ priv->event_thread[i].crtc = crtc;
priv->event_thread[i].dev = ddev;
priv->event_thread[i].worker = kthread_create_worker(0,
- "crtc_event:%d", priv->event_thread[i].crtc_id);
+ "crtc_event:%d", priv->event_thread[i].crtc->base.id);
if (IS_ERR(priv->event_thread[i].worker)) {
ret = PTR_ERR(priv->event_thread[i].worker);
[PATCH v2 2/3] drm/msm/mdp5: convert to drm_crtc_handle_vblank()
Stop using deprecated drm_handle_vblank(), use drm_crtc_handle_vblank() instead. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c index 9b4c8d92ff32..43443a435d59 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c @@ -82,8 +82,7 @@ irqreturn_t mdp5_irq(struct msm_kms *kms) struct mdp_kms *mdp_kms = to_mdp_kms(kms); struct mdp5_kms *mdp5_kms = to_mdp5_kms(mdp_kms); struct drm_device *dev = mdp5_kms->dev; - struct msm_drm_private *priv = dev->dev_private; - unsigned int id; + struct drm_crtc *crtc; uint32_t status, enable; enable = mdp5_read(mdp5_kms, REG_MDP5_INTR_EN); @@ -94,9 +93,9 @@ irqreturn_t mdp5_irq(struct msm_kms *kms) mdp_dispatch_irqs(mdp_kms, status); - for (id = 0; id < priv->num_crtcs; id++) - if (status & mdp5_crtc_vblank(priv->crtcs[id])) - drm_handle_vblank(dev, id); + drm_for_each_crtc(crtc, dev) + if (status & mdp5_crtc_vblank(crtc)) + drm_crtc_handle_vblank(crtc); return IRQ_HANDLED; } -- 2.35.1
[PATCH v2 1/3] drm/msm/mdp4: convert to drm_crtc_handle_vblank()
Stop using deprecated drm_handle_vblank(), use drm_crtc_handle_vblank() instead. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c index 4d49f3ba6a96..ddcdd5e87853 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c @@ -69,8 +69,7 @@ irqreturn_t mdp4_irq(struct msm_kms *kms) struct mdp_kms *mdp_kms = to_mdp_kms(kms); struct mdp4_kms *mdp4_kms = to_mdp4_kms(mdp_kms); struct drm_device *dev = mdp4_kms->dev; - struct msm_drm_private *priv = dev->dev_private; - unsigned int id; + struct drm_crtc *crtc; uint32_t status, enable; enable = mdp4_read(mdp4_kms, REG_MDP4_INTR_ENABLE); @@ -81,9 +80,9 @@ irqreturn_t mdp4_irq(struct msm_kms *kms) mdp_dispatch_irqs(mdp_kms, status); - for (id = 0; id < priv->num_crtcs; id++) - if (status & mdp4_crtc_vblank(priv->crtcs[id])) - drm_handle_vblank(dev, id); + drm_for_each_crtc(crtc, dev) + if (status & mdp4_crtc_vblank(crtc)) + drm_crtc_handle_vblank(crtc); return IRQ_HANDLED; } -- 2.35.1
Re: [PATCH] fbdev: efifb: Fix a use-after-free due early fb_info cleanup
Hello Lucas,
On 5/7/22 18:20, Lucas De Marchi wrote:
> On Fri, May 06, 2022 at 03:22:25PM +0200, Javier Martinez Canillas wrote:
>> Commit d258d00fb9c7 ("fbdev: efifb: Cleanup fb_info in .fb_destroy rather
>> than .remove") attempted to fix a use-after-free error due driver freeing
>> the fb_info in the .remove handler instead of doing it in .fb_destroy.
>>
>> But ironically that change introduced yet another use-after-free since the
>> fb_info was still used after the free.
>>
>> This should fix for good by freeing the fb_info at the end of the handler.
>>
>> Fixes: d258d00fb9c7 ("fbdev: efifb: Cleanup fb_info in .fb_destroy rather
>> than .remove")
>
> are these patches going through any CI before being applied? Maybe would
> be a good idea to cc intel-gfx mailing list on these fixes to have Intel
> CI to pick them up for some tests?
>
I Cc'ed intel-gfx for this particular patch. I should had done it for the
previous patches too, but I wasn't aware that Cc'ing that list would make
it run on your CI.
I tested locally the offending patch on an EFI platform before applying it
and I don't know why it didn't fail there. Sorry all for the inconvenience.
> pushed to drm-misc-fixes where the previous patch was applied.
>
Thanks.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH] fbdev: efifb: Fix a use-after-free due early fb_info cleanup
On Fri, May 06, 2022 at 03:22:25PM +0200, Javier Martinez Canillas wrote:
Commit d258d00fb9c7 ("fbdev: efifb: Cleanup fb_info in .fb_destroy rather
than .remove") attempted to fix a use-after-free error due driver freeing
the fb_info in the .remove handler instead of doing it in .fb_destroy.
But ironically that change introduced yet another use-after-free since the
fb_info was still used after the free.
This should fix for good by freeing the fb_info at the end of the handler.
Fixes: d258d00fb9c7 ("fbdev: efifb: Cleanup fb_info in .fb_destroy rather than
.remove")
are these patches going through any CI before being applied? Maybe would
be a good idea to cc intel-gfx mailing list on these fixes to have Intel
CI to pick them up for some tests?
pushed to drm-misc-fixes where the previous patch was applied.
thanks
LUcas De Marchi
Reported-by: Ville Syrjälä
Reported-by: Andrzej Hajda
Signed-off-by: Javier Martinez Canillas
---
drivers/video/fbdev/efifb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index cfa3dc0b4eee..b3d5f884c544 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -259,12 +259,12 @@ static void efifb_destroy(struct fb_info *info)
memunmap(info->screen_base);
}
- framebuffer_release(info);
-
if (request_mem_succeeded)
release_mem_region(info->apertures->ranges[0].base,
info->apertures->ranges[0].size);
fb_dealloc_cmap(>cmap);
+
+ framebuffer_release(info);
}
static const struct fb_ops efifb_ops = {
--
2.35.1
Re: [PATCH 2/3] dt-bindings: display: Add bindings for EBBG FT8719
On 06/05/2022 14:17, Joel Selvaraj wrote:
> Add bindings for the EBBG FT8719 6.18" 2246x1080 DSI video mode panel,
> which can be found on some Xiaomi Poco F1 phones. The backlight is
> managed through the QCOM WLED driver.
>
> Signed-off-by: Joel Selvaraj
> ---
> .../bindings/display/panel/ebbg,ft8719.yaml | 78 +++
> 1 file changed, 78 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/panel/ebbg,ft8719.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/panel/ebbg,ft8719.yaml
> b/Documentation/devicetree/bindings/display/panel/ebbg,ft8719.yaml
> new file mode 100644
> index ..fac6c9692c55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/ebbg,ft8719.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/ebbg,ft8719.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: EBBG FT8719 MIPI-DSI LCD panel
> +
> +maintainers:
> + - Joel Selvaraj
> +
> +description: |
> + The FT8719 panel from EBBG is a FHD+ LCD display panel with a resolution
> + of 1080x2246. It is a video mode DSI panel. The backlight is managed
> + through the QCOM WLED driver.
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> + compatible:
> +const: ebbg,ft8719
> +
> + reg:
> +description: DSI virtual channel of the peripheral
maxItems
> +
> + reset-gpios:
> +description: phandle of gpio for reset line
Skip description - it's obvious.
> +
> + vddio-supply:
> +description: phandle of the Power IC supply regulator
s/phandle of//
> +
> + vddpos-supply:
> +description: phandle of the positive boost supply regulator
> +
> + vddneg-supply:
> +description: phandle of the negative boost supply regulator
> +
> + backlight: true
> + port: true
Both should not be needed - they come from panel-common.yaml. They might
stay in list
> +
> +required:
> + - compatible
> + - reg
> + - vddio-supply
> + - vddpos-supply
> + - vddneg-supply
> + - reset-gpios
> + - port
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |+
No need for +
> +#include
> +dsi0 {
Just dsi
Best regards,
Krzysztof
Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add prefix for EBBG
On 06/05/2022 14:17, Joel Selvaraj wrote: > Add a prefix for EBBG. They manufacture displays which are used in some > Xiaomi phones, but I could not find much details about the company. > > Signed-off-by: Joel Selvaraj Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH 1/2] dma-buf: Add an API for exporting sync files (v14)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over-synchronization. By making this an ioctl on the dma-buf itself, it allows this new functionality to be used in an entirely driver-agnostic way without having access to a DRM fd. This makes it ideal for use in driver-generic code in Mesa or in a client such as a compositor where the DRM fd may be hard to reach. v2 (Jason Ekstrand): - Use a wrapper dma_fence_array of all fences including the new one when importing an exclusive fence. v3 (Jason Ekstrand): - Lock around setting shared fences as well as exclusive - Mark SIGNAL_SYNC_FILE as a read-write ioctl. - Initialize ret to 0 in dma_buf_wait_sync_file v4 (Jason Ekstrand): - Use the new dma_resv_get_singleton helper v5 (Jason Ekstrand): - Rename the IOCTLs to import/export rather than wait/signal - Drop the WRITE flag and always get/set the exclusive fence v6 (Jason Ekstrand): - Drop the sync_file import as it was all-around sketchy and not nearly as useful as import. - Re-introduce READ/WRITE flag support for export - Rework the commit message v7 (Jason Ekstrand): - Require at least one sync flag - Fix a refcounting bug: dma_resv_get_excl() doesn't take a reference - Use _rcu helpers since we're accessing the dma_resv read-only v8 (Jason Ekstrand): - Return -ENOMEM if the sync_file_create fails - Predicate support on IS_ENABLED(CONFIG_SYNC_FILE) v9 (Jason Ekstrand): - Add documentation for the new ioctl v10 (Jason Ekstrand): - Go back to dma_buf_sync_file as the ioctl struct name v11 (Daniel Vetter): - Go back to dma_buf_export_sync_file as the ioctl struct name - Better kerneldoc describing what the read/write flags do v12 (Christian König): - Document why we chose to make it an ioctl on dma-buf v13 (Jason Ekstrand): - Rebase on Christian König's fence rework v14 (Daniel Vetter & Christian König): - Use dma_rev_usage_rw to get the properly inverted usage to pass to dma_resv_get_singleton() - Clean up the sync_file and fd if copy_to_user() fails Signed-off-by: Jason Ekstrand Signed-off-by: Jason Ekstrand Signed-off-by: Jason Ekstrand Acked-by: Simon Ser Acked-by: Christian König Reviewed-by: Daniel Vetter Cc: Sumit Semwal Cc: Maarten Lankhorst --- drivers/dma-buf/dma-buf.c| 67 include/uapi/linux/dma-buf.h | 35 +++ 2 files changed, 102 insertions(+) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 79795857be3e..6ff54f7e6119 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -192,6 +193,9 @@ static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence) * Note that this only signals the completion of the respective fences, i.e. the * DMA transfers are complete. Cache flushing and any other necessary * preparations before CPU access can begin still need to happen. + * + * As an alternative to poll(), the set of fences on DMA buffer can be + * exported as a _file using _buf_sync_file_export. */ static void dma_buf_poll_cb(struct dma_fence *fence, struct dma_fence_cb *cb) @@ -326,6 +330,64 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf) return 0; } +#if IS_ENABLED(CONFIG_SYNC_FILE) +static long
[PATCH 0/2] dma-buf: Add an API for exporting sync files (v14)
Modern userspace APIs like Vulkan are built on an explicit synchronization model. This doesn't always play nicely with the implicit synchronization used in the kernel and assumed by X11 and Wayland. The client -> compositor half of the synchronization isn't too bad, at least on intel, because we can control whether or not i915 synchronizes on the buffer and whether or not it's considered written. The harder part is the compositor -> client synchronization when we get the buffer back from the compositor. We're required to be able to provide the client with a VkSemaphore and VkFence representing the point in time where the window system (compositor and/or display) finished using the buffer. With current APIs, it's very hard to do this in such a way that we don't get confused by the Vulkan driver's access of the buffer. In particular, once we tell the kernel that we're rendering to the buffer again, any CPU waits on the buffer or GPU dependencies will wait on some of the client rendering and not just the compositor. This new IOCTL solves this problem by allowing us to get a snapshot of the implicit synchronization state of a given dma-buf in the form of a sync file. It's effectively the same as a poll() or I915_GEM_WAIT only, instead of CPU waiting directly, it encapsulates the wait operation, at the current moment in time, in a sync_file so we can check/wait on it later. As long as the Vulkan driver does the sync_file export from the dma-buf before we re-introduce it for rendering, it will only contain fences from the compositor or display. This allows to accurately turn it into a VkFence or VkSemaphore without any over-synchronization. This patch series actually contains two new ioctls. There is the export one mentioned above as well as an RFC for an import ioctl which provides the other half. The intention is to land the export ioctl since it seems like there's no real disagreement on that one. The import ioctl, however, has a lot of debate around it so it's intended to be RFC-only for now. Mesa MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4037 IGT tests: https://patchwork.freedesktop.org/series/90490/ v10 (Jason Ekstrand, Daniel Vetter): - Add reviews/acks - Add a patch to rename _rcu to _unlocked - Split things better so import is clearly RFC status v11 (Daniel Vetter): - Add more CCs to try and get maintainers - Add a patch to document DMA_BUF_IOCTL_SYNC - Generally better docs - Use separate structs for import/export (easier to document) - Fix an issue in the import patch v12 (Daniel Vetter): - Better docs for DMA_BUF_IOCTL_SYNC v12 (Christian König): - Drop the rename patch in favor of Christian's series - Add a comment to the commit message for the dma-buf sync_file export ioctl saying why we made it an ioctl on dma-buf v13 (Jason Ekstrand): - Rebase on Christian König's fence rework v14 (Daniel Vetter and Christian König): - Use dma_rev_usage_rw to get the properly inverted usage to pass to dma_resv_get_singleton() for export - Clean up the sync_file and fd if copy_to_user() fails - Fix -EINVAL checks for the flags parameter in import - Add documentation about read/write fences for import - Add documentation about the expected usage of import/export and specifically call out the possible userspace race. Jason Ekstrand (2): dma-buf: Add an API for exporting sync files (v14) dma-buf: Add an API for importing sync files (v9) drivers/dma-buf/dma-buf.c| 106 +++ include/uapi/linux/dma-buf.h | 84 +++ 2 files changed, 190 insertions(+) -- 2.36.0
Re: [BUG] Warning and NULL-ptr dereference in amdgpu driver with 5.18
On Fri, May 06, 2022 at 08:30:13AM +0200, Jörg Rödel wrote: > [81829.087101] [ cut here ] > [81829.087105] WARNING: CPU: 4 PID: 644 at > drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dce110/dce110_clk_mgr.c:140 > dce110_fill_display_configs+0x4a/0x150 [amdgpu] Same just happened with a kernel built from latest upstream, based on commit fe27d189e3f42e31d3c8223d5daed7285e334c5e. So it's at least not the iommu changes causing it :) Please let me know if I can be of any help debugging this further. Thanks, Joerg
[PATCH 2/2] dma-buf: Add an API for importing sync files (v9)
This patch is analogous to the previous sync file export patch in that
it allows you to import a sync_file into a dma-buf. Unlike the previous
patch, however, this does add genuinely new functionality to dma-buf.
Without this, the only way to attach a sync_file to a dma-buf is to
submit a batch to your driver of choice which waits on the sync_file and
claims to write to the dma-buf. Even if said batch is a no-op, a submit
is typically way more overhead than just attaching a fence. A submit
may also imply extra synchronization with other work because it happens
on a hardware queue.
In the Vulkan world, this is useful for dealing with the out-fence from
vkQueuePresent. Current Linux window-systems (X11, Wayland, etc.) all
rely on dma-buf implicit sync. Since Vulkan is an explicit sync API, we
get a set of fences (VkSemaphores) in vkQueuePresent and have to stash
those as an exclusive (write) fence on the dma-buf. We handle it in
Mesa today with the above mentioned dummy submit trick. This ioctl
would allow us to set it directly without the dummy submit.
This may also open up possibilities for GPU drivers to move away from
implicit sync for their kernel driver uAPI and instead provide sync
files and rely on dma-buf import/export for communicating with other
implicit sync clients.
We make the explicit choice here to only allow setting RW fences which
translates to an exclusive fence on the dma_resv. There's no use for
read-only fences for communicating with other implicit sync userspace
and any such attempts are likely to be racy at best. When we got to
insert the RW fence, the actual fence we set as the new exclusive fence
is a combination of the sync_file provided by the user and all the other
fences on the dma_resv. This ensures that the newly added exclusive
fence will never signal before the old one would have and ensures that
we don't break any dma_resv contracts. We require userspace to specify
RW in the flags for symmetry with the export ioctl and in case we ever
want to support read fences in the future.
There is one downside here that's worth documenting: If two clients
writing to the same dma-buf using this API race with each other, their
actions on the dma-buf may happen in parallel or in an undefined order.
Both with and without this API, the pattern is the same: Collect all
the fences on dma-buf, submit work which depends on said fences, and
then set a new exclusive (write) fence on the dma-buf which depends on
said work. The difference is that, when it's all handled by the GPU
driver's submit ioctl, the three operations happen atomically under the
dma_resv lock. If two userspace submits race, one will happen before
the other. You aren't guaranteed which but you are guaranteed that
they're strictly ordered. If userspace manages the fences itself, then
these three operations happen separately and the two render operations
may happen genuinely in parallel or get interleaved. However, this is a
case of userspace racing with itself. As long as we ensure userspace
can't back the kernel into a corner, it should be fine.
v2 (Jason Ekstrand):
- Use a wrapper dma_fence_array of all fences including the new one
when importing an exclusive fence.
v3 (Jason Ekstrand):
- Lock around setting shared fences as well as exclusive
- Mark SIGNAL_SYNC_FILE as a read-write ioctl.
- Initialize ret to 0 in dma_buf_wait_sync_file
v4 (Jason Ekstrand):
- Use the new dma_resv_get_singleton helper
v5 (Jason Ekstrand):
- Rename the IOCTLs to import/export rather than wait/signal
- Drop the WRITE flag and always get/set the exclusive fence
v6 (Jason Ekstrand):
- Split import and export into separate patches
- New commit message
v7 (Daniel Vetter):
- Fix the uapi header to use the right struct in the ioctl
- Use a separate dma_buf_import_sync_file struct
- Add kerneldoc for dma_buf_import_sync_file
v8 (Jason Ekstrand):
- Rebase on Christian König's fence rework
v9 (Daniel Vetter):
- Fix -EINVAL checks for the flags parameter
- Add documentation about read/write fences
- Add documentation about the expected usage of import/export and
specifically call out the possible userspace race.
Signed-off-by: Jason Ekstrand
Signed-off-by: Jason Ekstrand
Signed-off-by: Jason Ekstrand
Cc: Christian König
Cc: Daniel Vetter
Cc: Sumit Semwal
Cc: Maarten Lankhorst
---
drivers/dma-buf/dma-buf.c| 39
include/uapi/linux/dma-buf.h | 49
2 files changed, 88 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 6ff54f7e6119..f23f1482eb38 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -386,6 +386,43 @@ static long dma_buf_export_sync_file(struct dma_buf
*dmabuf,
put_unused_fd(fd);
return ret;
}
+
+static long dma_buf_import_sync_file(struct dma_buf *dmabuf,
+const void __user *user_data)
+{
+ struct
Re: [PATCH v2 -next] drm/rockchip: Fix Kconfig dependencies
Hi Zhijie:
On 5/7/22 18:09, Ren Zhijie wrote:
drivers/gpu/drm/rockchip/cdn-dp-core.o: In function
`cdn_dp_connector_mode_valid':
cdn-dp-core.c:(.text+0x1e1): undefined reference to
`drm_dp_bw_code_to_link_rate'
cdn-dp-core.c:(.text+0x1f4): undefined reference to
`drm_dp_bw_code_to_link_rate'
drivers/gpu/drm/rockchip/cdn-dp-core.o: In function `cdn_dp_pd_event_work':
cdn-dp-core.c:(.text+0x138e): undefined reference to `drm_dp_channel_eq_ok'
drivers/gpu/drm/rockchip/cdn-dp-reg.o: In function `cdn_dp_train_link':
cdn-dp-reg.c:(.text+0xd5a): undefined reference to `drm_dp_bw_code_to_link_rate'
The DP-helper module has been replaced by the display-helper module.
So the driver have to select it.
Reported-by: Hulk Robot
Fixes: 1e0f66420b13("drm/display: Introduce a DRM display-helper module")
Signed-off-by: Ren Zhijie
Thanks.
Reviewed-by: Andy Yan
---
v2: remove "select DRM_DISPLAY_HELPER if ROCKCHIP_ANALOGIX_DP" under
DRM_ROCKCHIP at the head,
and separately add the select for ROCKCHIP_ANALOGIX_DP and ROCKCHIP_CDN_DP,
which Andy suggested.
---
---
drivers/gpu/drm/rockchip/Kconfig | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 5afab49dc4f2..53c2d9980d48 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -2,7 +2,6 @@
config DRM_ROCKCHIP
tristate "DRM Support for Rockchip"
depends on DRM && ROCKCHIP_IOMMU
- select DRM_DISPLAY_HELPER if ROCKCHIP_ANALOGIX_DP
select DRM_GEM_CMA_HELPER
select DRM_KMS_HELPER
select DRM_PANEL
@@ -38,6 +37,7 @@ config ROCKCHIP_VOP2
config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on ROCKCHIP_VOP
+ select DRM_DISPLAY_HELPER
select DRM_DISPLAY_DP_HELPER
help
This selects support for Rockchip SoC specific extensions
@@ -47,6 +47,8 @@ config ROCKCHIP_ANALOGIX_DP
config ROCKCHIP_CDN_DP
bool "Rockchip cdn DP"
depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m)
+ select DRM_DISPLAY_HELPER
+ select DRM_DISPLAY_DP_HELPER
help
This selects support for Rockchip SoC specific extensions
for the cdn DP driver. If you want to enable Dp on
Re: [PATCH 40/48] ARM: pxa: tosa: use gpio lookup for battery
On Tue, Apr 19, 2022 at 6:44 PM Arnd Bergmann wrote: > From: Arnd Bergmann > > The battery driver uses a lot of GPIO lines, hardcoded from a > machine header file. > > Change it to use a gpiod lookup table instead. > > Reviewed-by: Sebastian Reichel > Acked-by: Sebastian Reichel > Cc: linux...@vger.kernel.org > Signed-off-by: Arnd Bergmann Oh, I've been iterating a patch for the Tosa charging code going down in MFD ans ASoC and all: https://lore.kernel.org/linux-arm-kernel/20220125003741.492954-1-linus.wall...@linaro.org/ I just rebased this on v5.18-rc1 and resent with collected ACKs. Please take a look at it, and see if you rather take that patch, at some point I realized I had to go pretty deep around the legacy code in different subsystems because the MFD device us spawning a GPIO chip... Yours, Linus Walleij
Re: [PATCH 3/3] drm/msm: stop storing the array of CRTCs in struct msm_drm_private
Hi Dmitry,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on next-20220506]
[also build test WARNING on v5.18-rc5]
[cannot apply to drm/drm-next v5.18-rc5 v5.18-rc4 v5.18-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/Dmitry-Baryshkov/drm-msm-mdp4-convert-to-drm_crtc_handle_vblank/20220507-090522
base:38a288f5941ef03752887ad86f2d85442358c99a
config: hexagon-allyesconfig
(https://download.01.org/0day-ci/archive/20220507/202205072052.67ojktjd-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
af4cf1c6b8ed0d8102fc5e69acdc2fcbbcdaa9a7)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/08ab9442139f4b4c9e33ce35986014219fd1d5d0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Dmitry-Baryshkov/drm-msm-mdp4-convert-to-drm_crtc_handle_vblank/20220507-090522
git checkout 08ab9442139f4b4c9e33ce35986014219fd1d5d0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/msm/msm_drv.c:428:22: warning: variable 'i' is uninitialized
>> when used here [-Wuninitialized]
priv->event_thread[i].crtc = crtc;
^
drivers/gpu/drm/msm/msm_drv.c:353:12: note: initialize the variable 'i' to
silence this warning
int ret, i;
^
= 0
1 warning generated.
vim +/i +428 drivers/gpu/drm/msm/msm_drv.c
346
347 static int msm_drm_init(struct device *dev, const struct drm_driver
*drv)
348 {
349 struct msm_drm_private *priv = dev_get_drvdata(dev);
350 struct drm_device *ddev;
351 struct msm_kms *kms;
352 struct drm_crtc *crtc;
353 int ret, i;
354
355 if (drm_firmware_drivers_only())
356 return -ENODEV;
357
358 ddev = drm_dev_alloc(drv, dev);
359 if (IS_ERR(ddev)) {
360 DRM_DEV_ERROR(dev, "failed to allocate drm_device\n");
361 return PTR_ERR(ddev);
362 }
363 ddev->dev_private = priv;
364 priv->dev = ddev;
365
366 priv->wq = alloc_ordered_workqueue("msm", 0);
367 priv->hangcheck_period = DRM_MSM_HANGCHECK_DEFAULT_PERIOD;
368
369 INIT_LIST_HEAD(>objects);
370 mutex_init(>obj_lock);
371
372 INIT_LIST_HEAD(>inactive_willneed);
373 INIT_LIST_HEAD(>inactive_dontneed);
374 INIT_LIST_HEAD(>inactive_unpinned);
375 mutex_init(>mm_lock);
376
377 /* Teach lockdep about lock ordering wrt. shrinker: */
378 fs_reclaim_acquire(GFP_KERNEL);
379 might_lock(>mm_lock);
380 fs_reclaim_release(GFP_KERNEL);
381
382 drm_mode_config_init(ddev);
383
384 ret = msm_init_vram(ddev);
385 if (ret)
386 return ret;
387
388 /* Bind all our sub-components: */
389 ret = component_bind_all(dev, ddev);
390 if (ret)
391 return ret;
392
393 dma_set_max_seg_size(dev, UINT_MAX);
394
395 msm_gem_shrinker_init(ddev);
396
397 if (priv->kms_init) {
398 ret = priv->kms_init(ddev);
399 if (ret) {
400 DRM_DEV_ERROR(dev, "failed to load kms\n");
401 priv->kms = NULL;
402 goto err_msm_uninit;
403 }
404 kms = priv->kms;
405 } else {
406 /* valid only for the dummy headless case, where
of_node=NULL */
407 WARN_ON(dev->of_node);
408 kms = NULL;
409 }
410
411 /* Enable normalization of plane zpos */
412 ddev->mode_config.normalize_zpos = true;
413
414 if (kms) {
415 kms->dev = ddev;
416 ret = kms->funcs->hw_init(kms);
417
[PATCH 2/2] drm/msm/dpu: drop enum msm_display_caps
After the commit c46f0d69039c ("drm/msm: remove unused hotplug and edid
macros from msm_drv.h") the msm_display_caps enum contains two bits
describing whether the encoder should work in video or command mode.
Drop the enum and replace capabilities field in struct msm_display_info
with boolean is_cmd_mode field.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 31 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 +---
drivers/gpu/drm/msm/msm_drv.h | 10 ---
4 files changed, 16 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 07de0c0506d3..ce299b1e40a0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -636,7 +636,7 @@ static void _dpu_encoder_update_vsync_source(struct
dpu_encoder_virt *dpu_enc,
}
if (hw_mdptop->ops.setup_vsync_source &&
- disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) {
+ disp_info->is_cmd_mode) {
for (i = 0; i < dpu_enc->num_phys_encs; i++)
vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx;
@@ -720,8 +720,7 @@ static int dpu_encoder_resource_control(struct drm_encoder
*drm_enc,
}
dpu_enc = to_dpu_encoder_virt(drm_enc);
priv = drm_enc->dev->dev_private;
- is_vid_mode = dpu_enc->disp_info.capabilities &
- MSM_DISPLAY_CAP_VID_MODE;
+ is_vid_mode = !dpu_enc->disp_info.is_cmd_mode;
/*
* when idle_pc is not supported, process only KICKOFF, STOP and MODESET
@@ -1604,7 +1603,7 @@ void dpu_encoder_trigger_kickoff_pending(struct
drm_encoder *drm_enc)
/* update only for command mode primary ctl */
if ((phys == dpu_enc->cur_master) &&
- (disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE)
+ disp_info->is_cmd_mode
&& ctl->ops.trigger_pending)
ctl->ops.trigger_pending(ctl);
}
@@ -2141,20 +2140,19 @@ static int dpu_encoder_virt_add_phys_encs(
return -EINVAL;
}
- if (disp_info->capabilities & MSM_DISPLAY_CAP_VID_MODE) {
- enc = dpu_encoder_phys_vid_init(params);
+
+ if (disp_info->intf_type == DRM_MODE_ENCODER_VIRTUAL) {
+ enc = dpu_encoder_phys_wb_init(params);
if (IS_ERR(enc)) {
- DPU_ERROR_ENC(dpu_enc, "failed to init vid enc: %ld\n",
+ DPU_ERROR_ENC(dpu_enc, "failed to init wb enc: %ld\n",
PTR_ERR(enc));
return PTR_ERR(enc);
}
dpu_enc->phys_encs[dpu_enc->num_phys_encs] = enc;
++dpu_enc->num_phys_encs;
- }
-
- if (disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) {
+ } else if (disp_info->is_cmd_mode) {
enc = dpu_encoder_phys_cmd_init(params);
if (IS_ERR(enc)) {
@@ -2165,14 +2163,12 @@ static int dpu_encoder_virt_add_phys_encs(
dpu_enc->phys_encs[dpu_enc->num_phys_encs] = enc;
++dpu_enc->num_phys_encs;
- }
-
- if (disp_info->intf_type == DRM_MODE_ENCODER_VIRTUAL) {
- enc = dpu_encoder_phys_wb_init(params);
+ } else {
+ enc = dpu_encoder_phys_vid_init(params);
if (IS_ERR(enc)) {
- DPU_ERROR_ENC(dpu_enc, "failed to init wb enc: %ld\n",
- PTR_ERR(enc));
+ DPU_ERROR_ENC(dpu_enc, "failed to init vid enc: %ld\n",
+ PTR_ERR(enc));
return PTR_ERR(enc);
}
@@ -2232,8 +2228,7 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
DPU_DEBUG("dsi_info->num_of_h_tiles %d\n", disp_info->num_of_h_tiles);
- if ((disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) ||
- (disp_info->capabilities & MSM_DISPLAY_CAP_VID_MODE))
+ if (disp_info->intf_type != DRM_MODE_ENCODER_VIRTUAL)
dpu_enc->idle_pc_supported =
dpu_kms->catalog->caps->has_idle_pc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 781d41c91994..861870ac8ae7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -21,19 +21,19 @@
/**
* struct msm_display_info - defines display properties
* @intf_type: DRM_MODE_ENCODER_ type
- * @capabilities: Bitmask of display flags
* @num_of_h_tiles: Number of horizontal tiles in case of split interface
* @h_tile_instance:Controller instance used
[PATCH 1/2] drm/msm/dpu: dont_use IS_ERR_OR_NULL for encoder phys backends
The functions dpu_encoder_phys_foo_init() can not return NULL. Replace
corresponding IS_ERR_OR_NULL() checks with just IS_ERR().
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 52516eb20cb8..07de0c0506d3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2144,10 +2144,10 @@ static int dpu_encoder_virt_add_phys_encs(
if (disp_info->capabilities & MSM_DISPLAY_CAP_VID_MODE) {
enc = dpu_encoder_phys_vid_init(params);
- if (IS_ERR_OR_NULL(enc)) {
+ if (IS_ERR(enc)) {
DPU_ERROR_ENC(dpu_enc, "failed to init vid enc: %ld\n",
PTR_ERR(enc));
- return enc == NULL ? -EINVAL : PTR_ERR(enc);
+ return PTR_ERR(enc);
}
dpu_enc->phys_encs[dpu_enc->num_phys_encs] = enc;
@@ -2157,10 +2157,10 @@ static int dpu_encoder_virt_add_phys_encs(
if (disp_info->capabilities & MSM_DISPLAY_CAP_CMD_MODE) {
enc = dpu_encoder_phys_cmd_init(params);
- if (IS_ERR_OR_NULL(enc)) {
+ if (IS_ERR(enc)) {
DPU_ERROR_ENC(dpu_enc, "failed to init cmd enc: %ld\n",
PTR_ERR(enc));
- return enc == NULL ? -EINVAL : PTR_ERR(enc);
+ return PTR_ERR(enc);
}
dpu_enc->phys_encs[dpu_enc->num_phys_encs] = enc;
@@ -2170,10 +2170,10 @@ static int dpu_encoder_virt_add_phys_encs(
if (disp_info->intf_type == DRM_MODE_ENCODER_VIRTUAL) {
enc = dpu_encoder_phys_wb_init(params);
- if (IS_ERR_OR_NULL(enc)) {
+ if (IS_ERR(enc)) {
DPU_ERROR_ENC(dpu_enc, "failed to init wb enc: %ld\n",
PTR_ERR(enc));
- return enc == NULL ? -EINVAL : PTR_ERR(enc);
+ return PTR_ERR(enc);
}
dpu_enc->phys_encs[dpu_enc->num_phys_encs] = enc;
--
2.35.1
[PATCH] drm/msm/dpu: remove NULL-ness check in dpu_hw_intr_destroy
There is no need to check that kfree() argument is not NULL. Remove
extra check and call kfree() unconditionally.
Reported-by: kernel test robot
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index fa4f99034a08..915250b7f122 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -433,8 +433,7 @@ struct dpu_hw_intr *dpu_hw_intr_init(void __iomem *addr,
void dpu_hw_intr_destroy(struct dpu_hw_intr *intr)
{
- if (intr)
- kfree(intr);
+ kfree(intr);
}
int dpu_core_irq_register_callback(struct dpu_kms *dpu_kms, int irq_idx,
--
2.35.1
Re: [PATCH 17/25] drm/edid: add drm_edid helper for drm_edid_to_sad()
On Fri, 06 May 2022, Ville Syrjälä wrote:
> On Fri, May 06, 2022 at 01:10:24PM +0300, Jani Nikula wrote:
>> +int drm_edid_to_sad(const struct edid *edid, struct cea_sad **sads)
>> +{
>> +struct drm_edid drm_edid = {
>> +.edid = edid,
>> +.size = edid_size(edid),
>> +};
>> +
>> +return _drm_edid_to_sad(_edid, sads);
>
> No need to check for NULL edid in these wrappers?
Yeah, we do, and CI concurs. *facepalm*.
BR,
Jani.
>
>> +}
>> EXPORT_SYMBOL(drm_edid_to_sad);
>>
>> /**
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v2 0/4] drm/nvdla: Add driver support for NVDLA
On 02 5月 22 19:04:13, Thierry Reding wrote: > On Fri, Apr 29, 2022 at 11:28:10AM +0800, Cai Huoqing wrote: > > On 28 4月 22 18:56:07, Mikko Perttunen wrote: > > > On 4/28/22 17:10, Thierry Reding wrote: > > > > On Tue, Apr 26, 2022 at 02:07:57PM +0800, Cai Huoqing wrote: > > > > > The NVIDIA Deep Learning Accelerator (NVDLA) is an open source IP > > > > > which is integrated into NVIDIA Jetson AGX Xavier, > > > > > so add driver support for this accelerator." > > > > > > > > Hi, > > > > > > > > nice to see this work going on. For subsequent revisions, can you please > > > > also Cc the Tegra mailing list (linux-te...@vger.kernel.org) as well as > > > > the Tegra platform maintainers (that's Jon Hunter and myself). This will > > > > make sure that more people with an interest in this will see your work. > > > > Not everyone follows dri-devel, linaro-mm-sig or linux-media. > > > > > > > > Thanks, > > > > Thierry > > > > > > From a quick glance it looks like this driver pokes DLA hardware directly > > > which is not the intended programming model on Tegra hardware (there are > > > Falcon microcontrollers that offload task scheduling and synchronization > > > from the CPU). The hardware is also behind the Host1x bus so a simple > > > platform device is not sufficient. > > > > > > Was this driver developed against some platform with OpenDLA hardware > > > (i.e. > > > not Tegra)? > > > > > > If so, we'd need to verify if the hardware matches the hardware in > > > Tegra194. > > > Also, this driver may not be ideal for Tegra platforms since we would lack > > > the hardware scheduling and synchronization facilities. It is likely > > > necessary to have separate drivers for OpenDLA and Tegra's DLA > > > integration. > > > > > > Thanks, > > > Mikko > > > > > Tegra DLA seems to work with a slave coprocessor, the host driver just > > impelement message queue, share buffer, notification... The hardware > > detail of DLA maybe in the slave driver(not linux OS?). > > > > Sure, This driver just support for the SOCs or FPGAs that OPENDLA > > inside. I will change this kind of description "integrated into NVIDIA > > Jetson AGX Xavier" > > this driver dont support for Tegra directly. > > Yes, I think it would be good to make it clear that this is not going to > work with the Tegra instantiations so that people don't get confused. > > I think it would be ideal, though, if we could reuse as much of this > driver as possible to work with other instantiations. The only reference > to OpenDLA that I can find and which seems somehow relevant to this is > here: > > https://github.com/SCLUO/ITRI-OpenDLA Hi, thanks for your reply. the hardware code here, https://github.com/caihuoq/nvdla-hw or https://github.com/nvdla/hw which includes cmodel, RTL. I also make a docker image to run cmodel simulator(based on qemu) https://github.com/caihuoq/nvdla_docker It can be used to check this driver. Thanks, Cai > > Is that the version that you're using? Or is the version that you're > using at least compatible with that one? Apart from that and the Tegra > instantiations, are you aware of any other derivatives that we need to > account for? I'm worried that this might fragment to the point where it > becomes unmaintainable in upstream Linux. > > Even if this doesn't concern the Tegra instantiation, I think most of my > other comments remain valid. Things like global variables will get in > the way of multiple FPGA instantiations as well, for example. > > You will also need to provide the device tree bindings for the > particular instantiation that you're working on. Typically this would be > identified by a vendor-specific compatible string for your particular > board, but if it stems from a "canonical" FPGA mapping, matching on that > compatible string might also be an option. In either case, when you send > out the DT bindings, please include the devicet...@vger.kernel.org > mailing list so that they can be properly reviewed. > > Thierry
Re: [PATCH v5 8/9] drm: vkms: Adds XRGB_16161616 and ARGB_1616161616 formats
Hi
Am 04.04.22 um 22:45 schrieb Igor Torrente:
This will be useful to write tests that depends on these formats.
ARGB and XRGB follows the a similar implementation of the former formats.
Just adjusting for 16 bits per channel.
V3: Adapt the handlers to the new format introduced in patch 7 V3.
V5: Minor improvements
Added le16_to_cpu/cpu_to_le16 to the 16 bits color read/writes.
Is there something we could add to the DRM's format-conversion helpers?
Best regards
Thomas
Signed-off-by: Igor Torrente
---
drivers/gpu/drm/vkms/vkms_formats.c | 77 +++
drivers/gpu/drm/vkms/vkms_plane.c | 5 +-
drivers/gpu/drm/vkms/vkms_writeback.c | 2 +
3 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c
b/drivers/gpu/drm/vkms/vkms_formats.c
index 931a61405d6a..8d913fa7dbde 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -78,6 +78,41 @@ static void XRGB_to_argb_u16(struct line_buffer
*stage_buffer,
}
}
+static void ARGB16161616_to_argb_u16(struct line_buffer *stage_buffer,
+const struct vkms_frame_info *frame_info,
+int y)
+{
+ struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
+ u16 *src_pixels = get_packed_src_addr(frame_info, y);
+ int x, x_limit = min_t(size_t, drm_rect_width(_info->dst),
+ stage_buffer->n_pixels);
+
+ for (x = 0; x < x_limit; x++, src_pixels += 4) {
+ out_pixels[x].a = le16_to_cpu(src_pixels[3]);
+ out_pixels[x].r = le16_to_cpu(src_pixels[2]);
+ out_pixels[x].g = le16_to_cpu(src_pixels[1]);
+ out_pixels[x].b = le16_to_cpu(src_pixels[0]);
+ }
+}
+
+static void XRGB16161616_to_argb_u16(struct line_buffer *stage_buffer,
+const struct vkms_frame_info *frame_info,
+int y)
+{
+ struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
+ u16 *src_pixels = get_packed_src_addr(frame_info, y);
+ int x, x_limit = min_t(size_t, drm_rect_width(_info->dst),
+ stage_buffer->n_pixels);
+
+ for (x = 0; x < x_limit; x++, src_pixels += 4) {
+ out_pixels[x].a = (u16)0x;
+ out_pixels[x].r = le16_to_cpu(src_pixels[2]);
+ out_pixels[x].g = le16_to_cpu(src_pixels[1]);
+ out_pixels[x].b = le16_to_cpu(src_pixels[0]);
+ }
+}
+
+
/*
* The following functions take an line of argb_u16 pixels from the
* src_buffer, convert them to a specific format, and store them in the
@@ -130,12 +165,50 @@ static void argb_u16_to_XRGB(struct vkms_frame_info
*frame_info,
}
}
+static void argb_u16_to_ARGB16161616(struct vkms_frame_info *frame_info,
+const struct line_buffer *src_buffer, int
y)
+{
+ int x, x_dst = frame_info->dst.x1;
+ u16 *dst_pixels = packed_pixels_addr(frame_info, x_dst, y);
+ struct pixel_argb_u16 *in_pixels = src_buffer->pixels;
+ int x_limit = min_t(size_t, drm_rect_width(_info->dst),
+ src_buffer->n_pixels);
+
+ for (x = 0; x < x_limit; x++, dst_pixels += 4) {
+ dst_pixels[3] = cpu_to_le16(in_pixels[x].a);
+ dst_pixels[2] = cpu_to_le16(in_pixels[x].r);
+ dst_pixels[1] = cpu_to_le16(in_pixels[x].g);
+ dst_pixels[0] = cpu_to_le16(in_pixels[x].b);
+ }
+}
+
+static void argb_u16_to_XRGB16161616(struct vkms_frame_info *frame_info,
+const struct line_buffer *src_buffer, int
y)
+{
+ int x, x_dst = frame_info->dst.x1;
+ u16 *dst_pixels = packed_pixels_addr(frame_info, x_dst, y);
+ struct pixel_argb_u16 *in_pixels = src_buffer->pixels;
+ int x_limit = min_t(size_t, drm_rect_width(_info->dst),
+ src_buffer->n_pixels);
+
+ for (x = 0; x < x_limit; x++, dst_pixels += 4) {
+ dst_pixels[3] = (u8)0x;
+ dst_pixels[2] = cpu_to_le16(in_pixels[x].r);
+ dst_pixels[1] = cpu_to_le16(in_pixels[x].g);
+ dst_pixels[0] = cpu_to_le16(in_pixels[x].b);
+ }
+}
+
plane_format_transform_func get_plane_fmt_transform_function(u32 format)
{
if (format == DRM_FORMAT_ARGB)
return _to_argb_u16;
else if (format == DRM_FORMAT_XRGB)
return _to_argb_u16;
+ else if (format == DRM_FORMAT_ARGB16161616)
+ return _to_argb_u16;
+ else if (format == DRM_FORMAT_XRGB16161616)
+ return _to_argb_u16;
else
return NULL;
}
@@ -146,6 +219,10 @@ wb_format_transform_func get_wb_fmt_transform_function(u32
format)
return _u16_to_ARGB;
else if (format ==
Re: [PATCH RFC v6 00/21] DEPT(Dependency Tracker)
On Fri, May 06, 2022 at 09:11:35AM +0900, Byungchul Park wrote: > Linus wrote: > > > > On Wed, May 4, 2022 at 1:19 AM Byungchul Park > > wrote: > > > > > > Hi Linus and folks, > > > > > > I've been developing a tool for detecting deadlock possibilities by > > > tracking wait/event rather than lock(?) acquisition order to try to > > > cover all synchonization machanisms. > > > > So what is the actual status of reports these days? > > > > Last time I looked at some reports, it gave a lot of false positives > > due to mis-understanding prepare_to_sleep(). > > Yes, it was. I handled the case in the following way: > > 1. Stage the wait at prepare_to_sleep(), which might be used at commit. >Which has yet to be an actual wait that Dept considers. > 2. If the condition for sleep is true, the wait will be committed at >__schedule(). The wait becomes an actual one that Dept considers. > 3. If the condition is false and the task gets back to TASK_RUNNING, >clean(=reset) the staged wait. > > That way, Dept only works with what actually hits to __schedule() for > the waits through sleep. > > > For this all to make sense, it would need to not have false positives > > (or at least a very small number of them together with a way to sanely > > Yes. I agree with you. I got rid of them that way I described above. > IMHO DEPT should not report what lockdep allows (Not talking about wait events). I mean lockdep allows some kind of nested locks but DEPT reports them. When I was collecting reports from DEPT on varous configurations, Most of them was report of down_write_nested(), which is allowed in lockdep. DEPT should not report at least what we know it's not a real deadlock. Otherwise there will be reports that is never fixed, which is quite unpleasant and reporters cannot examine all of them if it's real deadlock or not. > > get rid of them), and also have a track record of finding things that > > lockdep doesn't. > > I have some reports that wait_for_completion or waitqueue is involved. > It's worth noting those are not tracked by Lockdep. I'm checking if > those are true positive or not. I will share those reports once I get > more convinced for that. > > > Maybe such reports have been sent out with the current situation, and > > I haven't seen them. > > Dept reports usually have been sent to me privately, not in LKML. As I > told you, I'm planning to share them. > > Byungchul > > > > > Linus > > -- Thanks, Hyeonggon
[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125
https://bugzilla.kernel.org/show_bug.cgi?id=205089 --- Comment #39 from MasterCATZ (masterc...@hotmail.com) --- h.264 is fine any h.265 does it do not know why my dmesg logs do not contain all the spam when the gpu resets -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH -next] drm/rockchip: Fix Kconfig dependencies
Hi Zhijie:
On 5/7/22 09:00, Ren Zhijie wrote:
If CONFIG_ROCKCHIP_ANALOGIX_DP is not set, the rockchip drm driver
will fail to link:
drivers/gpu/drm/rockchip/cdn-dp-core.o: In function
`cdn_dp_connector_mode_valid':
cdn-dp-core.c:(.text+0x1e1): undefined reference to
`drm_dp_bw_code_to_link_rate'
cdn-dp-core.c:(.text+0x1f4): undefined reference to
`drm_dp_bw_code_to_link_rate'
drivers/gpu/drm/rockchip/cdn-dp-core.o: In function `cdn_dp_pd_event_work':
cdn-dp-core.c:(.text+0x138e): undefined reference to `drm_dp_channel_eq_ok'
drivers/gpu/drm/rockchip/cdn-dp-reg.o: In function `cdn_dp_train_link':
cdn-dp-reg.c:(.text+0xd5a): undefined reference to `drm_dp_bw_code_to_link_rate'
The problem is that the DP-helper module has been replaced by the
display-helper module.
So the driver have to select it.
Reported-by: Hulk Robot
Fixes: 1e0f66420b13("drm/display: Introduce a DRM display-helper module")
Signed-off-by: Ren Zhijie
---
drivers/gpu/drm/rockchip/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 5afab49dc4f2..eb9ffa9e357d 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -47,6 +47,8 @@ config ROCKCHIP_ANALOGIX_DP
config ROCKCHIP_CDN_DP
bool "Rockchip cdn DP"
depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m)
+ select DRM_DISPLAY_HELPER
+ select DRM_DISPLAY_DP_HELPER
There are two dp(ANALOGIX_DP and CDN_DP) at rockchip drm mainline,
for a totally cleanup and alignment, I think it's better to remove
"select DRM_DISPLAY_HELPER if ROCKCHIP_ANALOGIX_DP" under DRM_ROCKCHIP
at the head,
and separately add the select for ROCKCHIP_ANALOGIX_DP and ROCKCHIP_CDN_DP.
help
This selects support for Rockchip SoC specific extensions
for the cdn DP driver. If you want to enable Dp on
[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125
https://bugzilla.kernel.org/show_bug.cgi?id=205089 --- Comment #38 from MasterCATZ (masterc...@hotmail.com) --- amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125 AMD Radeon R9 200 Series (hawaii, LLVM 14.0.0, DRM 3.42, 5.15.34-051534-generic) OpenGL version string: 4.6 (Compatibility Profile) Mesa 22.2.0-devel (git-6983c85 2022-05-07 impish-oibaf-ppa) Ubuntu 22.04 LTS Kernel command line: BOOT_IMAGE=/vmlinuz-5.15.34-051534-generic root=/dev/mapper/Raid6LVM-lvUbuntu ro rootflags=subvol=@ amdgpu.gpu_recovery=1 amd_iommu=on iommu=pt delayacct acpi_enforce_resources=lax usbcore.autosuspend=-1 apparmor=0 amdgpu.dc=1 amdgpu.dpm=1 amdgpu.ppfeaturemask=0xfffd7fff amdgpu.dcfeaturemask=2 amdgpu.si_support=1 amdgpu.cik_support=1 radeon.si_support=0 I could not find my dmesg logs containing the crash and neither did journalctl -k --since "2 hours ago" -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
