Re: [PATCH v2] drm/atomic-helpers: Invoke end_fb_access while owning plane state
Hi
Am 27.11.23 um 17:25 schrieb Alyssa Ross:
Thomas Zimmermann writes:
Invoke drm_plane_helper_funcs.end_fb_access before
drm_atomic_helper_commit_hw_done(). The latter function hands over
ownership of the plane state to the following commit, which might
free it. Releasing resources in end_fb_access then operates on undefined
state. This bug has been observed with non-blocking commits when they
are being queued up quickly.
Here is an example stack trace from the bug report. The plane state has
been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
Unable to handle kernel paging request at virtual address 00010049
[...]
drm_gem_fb_vunmap+0x18/0x74
drm_gem_end_shadow_fb_access+0x1c/0x2c
drm_atomic_helper_cleanup_planes+0x58/0xd8
drm_atomic_helper_commit_tail+0x90/0xa0
commit_tail+0x15c/0x188
commit_work+0x14/0x20
For aborted commits, it is still ok to run end_fb_access as part of the
plane's cleanup. Add a test to drm_atomic_helper_cleanup_planes().
v2:
* fix test in drm_atomic_helper_cleanup_planes()
Reported-by: Alyssa Ross
Closes: https://lore.kernel.org/dri-devel/87leazm0ya@alyssa.is/
Suggested-by: Daniel Vetter
Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane
helpers")
Signed-off-by: Thomas Zimmermann
Cc: # v6.2+
---
drivers/gpu/drm/drm_atomic_helper.c | 17 +
1 file changed, 17 insertions(+)
Got this basically immediately. :(
I've never seen such problems on other systems. Is there anything
different about the Mac systems? How do you trigger these errors?
Best regards
Thomas
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_init] Allocated
atomic state cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_plane_state]
Added [PLANE:31:plane-0] 4935bdca state to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_crtc_state] Added
[CRTC:33:crtc-0] d25f613d state to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_set_fb_for_plane] Set
[FB:38] for [PLANE:31:plane-0] state 4935bdca
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_connector_state]
Added [CONNECTOR:35:Unknown-1] 20d19f10 state to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_check_only] checking
cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
Updating routing for [CONNECTOR:35:Unknown-1]
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
[CONNECTOR:35:Unknown-1] keeps [ENCODER:34:None-34], now on [CRTC:33:crtc-0]
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_nonblocking_commit]
committing cfb3f1f2 nonblocking
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_default_clear]
Clearing atomic state cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:__drm_atomic_state_free] Freeing
atomic state cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_init] Allocated
atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_plane_state]
Added [PLANE:31:plane-0] 83f22dc6 state to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_crtc_state] Added
[CRTC:33:crtc-0] eec339c5 state to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_set_fb_for_plane] Set
[FB:37] for [PLANE:31:plane-0] state 83f22dc6
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_connector_state]
Added [CONNECTOR:35:Unknown-1] 22495ce9 state to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_check_only] checking
03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
Updating routing for [CONNECTOR:35:Unknown-1]
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
[CONNECTOR:35:Unknown-1] keeps [ENCODER:34:None-34], now on [CRTC:33:crtc-0]
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_default_clear]
Clearing atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:__drm_atomic_state_free] Freeing
atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_init] Allocated
atomic state 03dc0c0b
Re: [PATCH] drm: lcdif: Switch to drmm_mode_config_init
Am 27.11.23 um 22:26 schrieb Marek Vasut:
Switch from deprecated unmanaged drm_mode_config_init() to
managed drmm_mode_config_init(). No functional change.
Signed-off-by: Marek Vasut
---
Cc: Daniel Vetter
Cc: David Airlie
Cc: Fabio Estevam
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: NXP Linux Team
Cc: Pengutronix Kernel Team
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: Stefan Agner
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-ker...@lists.infradead.org
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/mxsfb/lcdif_drv.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 18de2f17e2491..ea10bf81582e9 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -167,7 +167,11 @@ static int lcdif_load(struct drm_device *drm)
return ret;
/* Modeset init */
- drm_mode_config_init(drm);
+ ret = drmm_mode_config_init(drm);
+ if (ret) {
+ dev_err(drm->dev, "Failed to initialize mode config\n");
+ return ret;
+ }
ret = lcdif_kms_init(lcdif);
if (ret < 0) {
@@ -227,7 +231,6 @@ static void lcdif_unload(struct drm_device *drm)
drm_crtc_vblank_off(>crtc);
drm_kms_helper_poll_fini(drm);
- drm_mode_config_cleanup(drm);
pm_runtime_put_sync(drm->dev);
pm_runtime_disable(drm->dev);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2] drm/mxsfb: Switch to drmm_mode_config_init
Am 27.11.23 um 22:25 schrieb Marek Vasut:
Switch from deprecated unmanaged drm_mode_config_init() to
managed drmm_mode_config_init(). No functional change.
Signed-off-by: Marek Vasut
---
Cc: Daniel Vetter
Cc: David Airlie
Cc: Fabio Estevam
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: NXP Linux Team
Cc: Pengutronix Kernel Team
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: Stefan Agner
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-ker...@lists.infradead.org
Acked-by: Thomas Zimmermann
---
V2: Drop matching drm_mode_config_cleanup
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 3bfa369b2507e..cfa3176ebe2c2 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -248,7 +248,11 @@ static int mxsfb_load(struct drm_device *drm,
pm_runtime_enable(drm->dev);
/* Modeset init */
- drm_mode_config_init(drm);
+ ret = drmm_mode_config_init(drm);
+ if (ret) {
+ dev_err(drm->dev, "Failed to initialize mode config\n");
+ goto err_vblank;
+ }
ret = mxsfb_kms_init(mxsfb);
if (ret < 0) {
@@ -311,7 +315,6 @@ static int mxsfb_load(struct drm_device *drm,
static void mxsfb_unload(struct drm_device *drm)
{
drm_kms_helper_poll_fini(drm);
- drm_mode_config_cleanup(drm);
pm_runtime_get_sync(drm->dev);
mxsfb_irq_uninstall(drm);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH] PCI: qcom: Fix compile error
On 28/11/23 12:21, Manivannan Sadhasivam wrote:
On Tue, Nov 28, 2023 at 11:44:26AM +0530, Vignesh Raman wrote:
Hi Mani,
On 28/11/23 10:44, Manivannan Sadhasivam wrote:
On Tue, Nov 28, 2023 at 09:50:26AM +0530, Vignesh Raman wrote:
Commit a2458d8f618a ("PCI/ASPM: pci_enable_link_state: Add argument
to acquire bus lock") has added an argument to acquire bus lock
in pci_enable_link_state, but qcom_pcie_enable_aspm calls it
without this argument, resulting in below build error.
Where do you see this error? That patch is not even merged. Looks like you are
sending the patch against some downstream tree.
I got this error with drm-tip - git://anongit.freedesktop.org/drm-tip
This commit is merged in drm-intel/topic/core-for-CI -
https://cgit.freedesktop.org/drm-intel/log/?h=topic/core-for-CI
Okay. Since this patch is just for CI, please do not CC linux-pci as it causes
confusion.
Sure, thank you.
Jani, is this fix required for topic/core-for-CI ?
Regards,
Vignesh
Re: [PATCH 01/14] arch/powerpc: Remove legacy DRM drivers from default configs
On 22 11月 23 13:09:30, Thomas Zimmermann wrote:
> DRM drivers for user-space modesetting have been removed. Do not
> select the respective options in the default configs.
>
> Signed-off-by: Thomas Zimmermann
> Fixes: a276afc19eec ("drm: Remove some obsolete drm pciids(tdfx, mga, i810,
> savage, r128, sis, via)")
Reviewed-by: Cai Huoqing
> Cc: Cai Huoqing
> Cc: Daniel Vetter
> Cc: Dave Airlie
> Cc: Thomas Zimmermann
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: dri-devel@lists.freedesktop.org
> Cc: # v6.3+
> ---
> arch/powerpc/configs/pmac32_defconfig | 2 --
> arch/powerpc/configs/ppc6xx_defconfig | 7 ---
> 2 files changed, 9 deletions(-)
>
> diff --git a/arch/powerpc/configs/pmac32_defconfig
> b/arch/powerpc/configs/pmac32_defconfig
> index 57ded82c28409..e41e7affd2482 100644
> --- a/arch/powerpc/configs/pmac32_defconfig
> +++ b/arch/powerpc/configs/pmac32_defconfig
> @@ -188,8 +188,6 @@ CONFIG_AGP=m
> CONFIG_AGP_UNINORTH=m
> CONFIG_DRM=m
> CONFIG_DRM_RADEON=m
> -CONFIG_DRM_LEGACY=y
> -CONFIG_DRM_R128=m
> CONFIG_FB=y
> CONFIG_FB_OF=y
> CONFIG_FB_CONTROL=y
> diff --git a/arch/powerpc/configs/ppc6xx_defconfig
> b/arch/powerpc/configs/ppc6xx_defconfig
> index f279703425d45..e680cd086f0e8 100644
> --- a/arch/powerpc/configs/ppc6xx_defconfig
> +++ b/arch/powerpc/configs/ppc6xx_defconfig
> @@ -678,13 +678,6 @@ CONFIG_AGP=y
> CONFIG_AGP_UNINORTH=y
> CONFIG_DRM=m
> CONFIG_DRM_RADEON=m
> -CONFIG_DRM_LEGACY=y
> -CONFIG_DRM_TDFX=m
> -CONFIG_DRM_R128=m
> -CONFIG_DRM_MGA=m
> -CONFIG_DRM_SIS=m
> -CONFIG_DRM_VIA=m
> -CONFIG_DRM_SAVAGE=m
> CONFIG_FB=y
> CONFIG_FB_CIRRUS=m
> CONFIG_FB_OF=y
> --
> 2.42.1
>
[bug report] backlight: mp3309c: Add support for MPS MP3309C
Hello Flavio Suligoi,
The patch 2e914516a58c: "backlight: mp3309c: Add support for MPS
MP3309C" from Nov 16, 2023 (linux-next), leads to the following
Smatch static checker warning:
drivers/video/backlight/mp3309c.c:277 pm3309c_parse_dt_node()
error: uninitialized symbol 'prop_levels'.
drivers/video/backlight/mp3309c.c
202 static int pm3309c_parse_dt_node(struct mp3309c_chip *chip,
203 struct mp3309c_platform_data *pdata)
204 {
205 struct device_node *node = chip->dev->of_node;
206 struct property *prop_pwms, *prop_levels;
207 int length = 0;
208 int ret, i;
209 unsigned int num_levels, tmp_value;
210
211 if (!node) {
212 dev_err(chip->dev, "failed to get DT node\n");
213 return -ENODEV;
214 }
215
216 /*
217 * Dimming mode: the MP3309C provides two dimming control mode:
218 *
219 * - PWM mode
220 * - Analog by I2C control mode (default)
221 *
222 * I2C control mode is assumed as default but, if the pwms
property is
223 * found in the backlight node, the mode switches to PWM mode.
224 */
225 pdata->dimming_mode = DIMMING_ANALOG_I2C;
226 prop_pwms = of_find_property(node, "pwms", );
227 if (prop_pwms) {
228 chip->pwmd = devm_pwm_get(chip->dev, NULL);
229 if (IS_ERR(chip->pwmd))
230 return dev_err_probe(chip->dev,
PTR_ERR(chip->pwmd),
231 "error getting pwm
data\n");
232 pdata->dimming_mode = DIMMING_PWM;
233 pwm_apply_args(chip->pwmd);
234 }
235
236 /*
237 * In I2C control mode the dimming levels (0..31) are fixed by
the
238 * hardware, while in PWM control mode they can be chosen by
the user,
239 * to allow nonlinear mappings.
240 */
241 if (pdata->dimming_mode == DIMMING_ANALOG_I2C) {
242 /*
243 * Analog (by I2C commands) control mode: fixed 0..31
brightness
244 * levels
245 */
246 num_levels = ANALOG_I2C_NUM_LEVELS;
247
248 /* Enable GPIO used in I2C dimming mode only */
249 chip->enable_gpio = devm_gpiod_get(chip->dev, "enable",
250GPIOD_OUT_HIGH);
251 if (IS_ERR(chip->enable_gpio))
252 return dev_err_probe(chip->dev,
253 PTR_ERR(chip->enable_gpio),
254 "error getting enable
gpio\n");
prop_levels not initialized on this path.
255 } else {
256 /*
257 * PWM control mode: check for brightness level in DT
258 */
259 prop_levels = of_find_property(node,
"brightness-levels",
260);
261 if (prop_levels) {
262 /* Read brightness levels from DT */
263 num_levels = length / sizeof(u32);
264 if (num_levels < 2)
265 return -EINVAL;
266 } else {
267 /* Use default brightness levels */
268 num_levels = MP3309C_PWM_DEFAULT_NUM_LEVELS;
269 }
270 }
271
272 /* Fill brightness levels array */
273 pdata->levels = devm_kcalloc(chip->dev, num_levels,
274 sizeof(*pdata->levels),
GFP_KERNEL);
275 if (!pdata->levels)
276 return -ENOMEM;
--> 277 if (prop_levels) {
^^^
Uninitialized
278 ret = of_property_read_u32_array(node,
"brightness-levels",
279 pdata->levels,
280 num_levels);
281 if (ret < 0)
282 return ret;
283 } else {
284 for (i = 0; i < num_levels; i++)
285 pdata->levels[i] = i;
286 }
287
288 pdata->max_brightness = num_levels - 1;
289
290 ret = of_property_read_u32(node, "default-brightness",
291>default_brightness);
292 if (ret)
293 pdata->default_brightness = pdata->max_brightness;
294 if
Re: [PATCH] PCI: qcom: Fix compile error
On Tue, Nov 28, 2023 at 11:44:26AM +0530, Vignesh Raman wrote:
> Hi Mani,
>
> On 28/11/23 10:44, Manivannan Sadhasivam wrote:
> > On Tue, Nov 28, 2023 at 09:50:26AM +0530, Vignesh Raman wrote:
> > > Commit a2458d8f618a ("PCI/ASPM: pci_enable_link_state: Add argument
> > > to acquire bus lock") has added an argument to acquire bus lock
> > > in pci_enable_link_state, but qcom_pcie_enable_aspm calls it
> > > without this argument, resulting in below build error.
> > >
> >
> > Where do you see this error? That patch is not even merged. Looks like you
> > are
> > sending the patch against some downstream tree.
>
> I got this error with drm-tip - git://anongit.freedesktop.org/drm-tip
>
> This commit is merged in drm-intel/topic/core-for-CI -
> https://cgit.freedesktop.org/drm-intel/log/?h=topic/core-for-CI
>
Okay. Since this patch is just for CI, please do not CC linux-pci as it causes
confusion.
- Mani
> Regards,
> Vignesh
--
மணிவண்ணன் சதாசிவம்
Re: [PATCH] PCI: qcom: Fix compile error
Hi Mani,
On 28/11/23 10:44, Manivannan Sadhasivam wrote:
On Tue, Nov 28, 2023 at 09:50:26AM +0530, Vignesh Raman wrote:
Commit a2458d8f618a ("PCI/ASPM: pci_enable_link_state: Add argument
to acquire bus lock") has added an argument to acquire bus lock
in pci_enable_link_state, but qcom_pcie_enable_aspm calls it
without this argument, resulting in below build error.
Where do you see this error? That patch is not even merged. Looks like you are
sending the patch against some downstream tree.
I got this error with drm-tip - git://anongit.freedesktop.org/drm-tip
This commit is merged in drm-intel/topic/core-for-CI -
https://cgit.freedesktop.org/drm-intel/log/?h=topic/core-for-CI
Regards,
Vignesh
Re: [PATCH] PCI: qcom: Fix compile error
On Tue, Nov 28, 2023 at 09:50:26AM +0530, Vignesh Raman wrote:
> Commit a2458d8f618a ("PCI/ASPM: pci_enable_link_state: Add argument
> to acquire bus lock") has added an argument to acquire bus lock
> in pci_enable_link_state, but qcom_pcie_enable_aspm calls it
> without this argument, resulting in below build error.
>
Where do you see this error? That patch is not even merged. Looks like you are
sending the patch against some downstream tree.
- Mani
> drivers/pci/controller/dwc/pcie-qcom.c:973:9: error: too few arguments to
> function 'pci_enable_link_state'
>
> This commit fixes the compilation error by passing the sem argument
> to pci_enable_link_state in the qcom_pcie_enable_aspm function.
>
> Signed-off-by: Vignesh Raman
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6902e97719d1..e846e3531d8e 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -970,7 +970,7 @@ static int qcom_pcie_enable_aspm(struct pci_dev *pdev,
> void *userdata)
> {
> /* Downstream devices need to be in D0 state before enabling PCI PM
> substates */
> pci_set_power_state(pdev, PCI_D0);
> - pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
> + pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL, false);
>
> return 0;
> }
> --
> 2.40.1
>
>
--
மணிவண்ணன் சதாசிவம்
[PATCH] PCI: qcom: Fix compile error
Commit a2458d8f618a ("PCI/ASPM: pci_enable_link_state: Add argument
to acquire bus lock") has added an argument to acquire bus lock
in pci_enable_link_state, but qcom_pcie_enable_aspm calls it
without this argument, resulting in below build error.
drivers/pci/controller/dwc/pcie-qcom.c:973:9: error: too few arguments to
function 'pci_enable_link_state'
This commit fixes the compilation error by passing the sem argument
to pci_enable_link_state in the qcom_pcie_enable_aspm function.
Signed-off-by: Vignesh Raman
---
drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
b/drivers/pci/controller/dwc/pcie-qcom.c
index 6902e97719d1..e846e3531d8e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -970,7 +970,7 @@ static int qcom_pcie_enable_aspm(struct pci_dev *pdev, void
*userdata)
{
/* Downstream devices need to be in D0 state before enabling PCI PM
substates */
pci_set_power_state(pdev, PCI_D0);
- pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
+ pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL, false);
return 0;
}
--
2.40.1
[PATCH v2] drm/exynos: fix a wrong error checking
Fix a wrong error checking in exynos_drm_dma.c module.
In the exynos_drm_register_dma function, both arm_iommu_create_mapping()
and iommu_get_domain_for_dev() functions are expected to return NULL as
an error.
However, the error checking is performed using the statement
if(IS_ERR(mapping)), which doesn't provide a suitable error value.
So check if 'mapping' is NULL, and if it is, return -ENODEV.
This issue[1] was reported by Dan.
Changelog v1:
- fix build warning.
[1]
https://lore.kernel.org/all/33e52277-1349-472b-a55b-ab5c3462bfcf@moroto.mountain/
Reported-by : Dan Carpenter
Signed-off-by: Inki Dae
---
drivers/gpu/drm/exynos/exynos_drm_dma.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c
b/drivers/gpu/drm/exynos/exynos_drm_dma.c
index a971590b8132..e2c7373f20c6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
@@ -107,18 +107,16 @@ int exynos_drm_register_dma(struct drm_device *drm,
struct device *dev,
return 0;
if (!priv->mapping) {
- void *mapping;
+ void *mapping = NULL;
if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU))
mapping = arm_iommu_create_mapping(_bus_type,
EXYNOS_DEV_ADDR_START, EXYNOS_DEV_ADDR_SIZE);
else if (IS_ENABLED(CONFIG_IOMMU_DMA))
mapping = iommu_get_domain_for_dev(priv->dma_dev);
- else
- mapping = ERR_PTR(-ENODEV);
- if (IS_ERR(mapping))
- return PTR_ERR(mapping);
+ if (!mapping)
+ return -ENODEV;
priv->mapping = mapping;
}
--
2.25.1
Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys_base to be relative not absolute
On Mon, Nov 27, 2023 at 8:20 PM Paz Zcharya wrote: > > On 21.11.2023 13:06, Andrzej Hajda wrote: > > On 18.11.2023 00:01, Paz Zcharya wrote: > > > On Tue, Nov 14, 2023 at 10:13:59PM -0500, Rodrigo Vivi wrote: > > > > On Sun, Nov 05, 2023 at 05:27:03PM +, Paz Zcharya wrote: > > > > > > Hi Rodrigo, thanks for the great comments. > > > > > > Apologies for using a wrong/confusing terminology. I think 'phys_base' > > > is supposed to be the offset in the GEM BO, where base (or > > > "Surface Base Address") is supposed to be the GTT offset. > > > > Since base is taken from PLANE_SURF register it should be resolvable via > > GGTT to physical address pointing to actual framebuffer. > > I couldn't find anything in the specs. > > It was quite cryptic. I meant I have not found anything about assumption > from commit history that for iGPU there should be 1:1 mapping, this is why > there was an assignment "phys_base = base". Possibly the assumption is not > valid anymore for MTL(?). > Without the assumption we need to check GGTT to determine phys address. > > > The simplest approach would be then do the same as in case of DGFX: > > gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; > > gen8_pte_t pte; > > > > gte += base / I915_GTT_PAGE_SIZE; > > > > pte = ioread64(gte); > > phys_base = pte & I915_GTT_PAGE_MASK; > > > > Regards > > Andrzej Hey Andrzej, On a second thought, what do you think about something like + gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; + gen8_pte_t pte; + gte += base / I915_GTT_PAGE_SIZE; + pte = ioread64(gte); + pte = pte & I915_GTT_PAGE_MASK; + phys_base = pte - i915->mm.stolen_region->region.start; The only difference is the last line. Based on what I wrote before, I think `phys_base` is named incorrectly and that it does not reflect the physical address, but the start offset of i915->mm.stolen_region. So if we offset the start value of the stolen region, this code looks correct to me (and it also works on my MeteorLake device). What do you think? Many thanks, Paz
Re: [v3 3/3] drm/bridge: it6505: Add audio support
On Tue, Nov 21, 2023 at 8:54 PM AngeloGioacchino Del Regno
wrote:
>
> Il 30/07/23 20:08, Jiaxin Yu ha scritto:
> > Add audio support for it6505
> >
> > 1. Bridge to hdmi-codec to support audio feature. At the same time,
> > the function of automatically detecting audio is removed.
> > 2. It is observed that some DP-to-HDMI dongles will get into bad
> > states if sending InfoFrame without audio data. Defer to enable
> > it6505's audio feature when PCM triggers START or RESUME.
> >
> > Signed-off-by: Jiaxin Yu
>
> Hello Jiaxin,
> this patch doesn't apply anymore (and it won't build anymore) upstream.
>
> > ---
> > drivers/gpu/drm/bridge/ite-it6505.c | 81 ++---
> > 1 file changed, 75 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c
> > b/drivers/gpu/drm/bridge/ite-it6505.c
> > index 504d51c42f79..1cfcb0731288 100644
> > --- a/drivers/gpu/drm/bridge/ite-it6505.c
> > +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> > @@ -2162,7 +2162,6 @@ static void it6505_stop_link_train(struct it6505
> > *it6505)
> >
> > static void it6505_link_train_ok(struct it6505 *it6505)
> > {
> > - struct device *dev = >client->dev;
>
> This is because this changed to `struct device *dev = it6505->dev;`
>
> >
> > it6505->link_state = LINK_OK;
> > /* disalbe mute enable avi info frame */
> > @@ -2170,11 +2169,6 @@ static void it6505_link_train_ok(struct it6505
> > *it6505)
> > it6505_set_bits(it6505, REG_INFOFRAME_CTRL,
> > EN_VID_CTRL_PKT, EN_VID_CTRL_PKT);
> >
> > - if (it6505_audio_input(it6505)) {
> > - DRM_DEV_DEBUG_DRIVER(dev, "Enable audio!");
> > - it6505_enable_audio(it6505);
> > - }
> > -
> > if (it6505->hdcp_desired)
> > it6505_start_hdcp(it6505);
> > }
> > @@ -2846,6 +2840,45 @@ static void __maybe_unused
> > it6505_audio_shutdown(struct device *dev, void *data)
> > it6505_disable_audio(it6505);
> > }
> >
> > +static int it6505_audio_hw_params(struct device *dev, void *data,
> > + struct hdmi_codec_daifmt *daifmt,
> > + struct hdmi_codec_params *params)
> > +{
> > + struct it6505 *it6505 = dev_get_drvdata(dev);
> > +
> > + return it6505_audio_setup_hw_params(it6505, params);
> > +}
> > +
> > +static int it6505_audio_setup_trigger(struct it6505 *it6505, int cmd)
> > +{
> > + struct device *dev = >client->dev;
>
> ...and because you'll have to change this one, and other occurrences of that
> as well.
>
> Can you please respin this series?
Please also add a patch adding #sound-dai-cells to the it6505 binding.
Re: [PATCH] drm/msm/mdp4: flush vblank event on disable
On 11/27/2023 3:17 PM, Dmitry Baryshkov wrote:
On Tue, 28 Nov 2023 at 00:00, Abhinav Kumar wrote:
On 11/27/2023 1:54 PM, Dmitry Baryshkov wrote:
Flush queued events when disabling the crtc. This avoids timeouts when
we come back and wait for dependencies (like the previous frame's
flip_done).
Fixes: c8afe684c95c ("drm/msm: basic KMS driver for snapdragon")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 169f9de4a12a..3100957225a7 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -269,6 +269,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
{
struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
struct mdp4_kms *mdp4_kms = get_kms(crtc);
+ unsigned long flags;
DBG("%s", mdp4_crtc->name);
@@ -281,6 +282,14 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
mdp_irq_unregister(_kms->base, _crtc->err);
mdp4_disable(mdp4_kms);
+ if (crtc->state->event && !crtc->state->active) {
+ WARN_ON(mdp4_crtc->event);
Do you need a WARN_ON() here? Just wondering how often this might happen
and spam.
Well, it is atomic_disable(), so it happens only on modesets or output
disable. Also, this is a port of mdp5 code, see commit e765ea77b0fd
("drm/msm: Issue queued events when disabling crtc"). We don't have
reports from MDP5 code, so there should be no comparable reports for
MDP4.
Ok, I thought this was ported from dpu code, even that has the same
logic but not the WARN_ON().
But I am fine with this,
Reviewed-by: Abhinav Kumar
otherwise LGTM.
+ spin_lock_irqsave(_kms->dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(_kms->dev->event_lock, flags);
+ }
+
mdp4_crtc->enabled = false;
}
Re: [PATCH] drm/msm/dpu: Capture dpu snapshot when frame_done_timer timeouts
On Tue, 28 Nov 2023 at 03:12, Paloma Arellano wrote:
>
> Trigger a devcoredump to dump dpu registers and capture the drm atomic
> state when the frame_done_timer timeouts.
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1cf7ff6caff4..5cf7594feb5a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -191,6 +191,7 @@ struct dpu_encoder_virt {
> void *crtc_frame_event_cb_data;
>
> atomic_t frame_done_timeout_ms;
> + atomic_t frame_done_timeout_cnt;
> struct timer_list frame_done_timer;
>
> struct msm_display_info disp_info;
> @@ -1204,6 +1205,8 @@ static void dpu_encoder_virt_atomic_enable(struct
> drm_encoder *drm_enc,
>
> dpu_enc->dsc = dpu_encoder_get_dsc_config(drm_enc);
>
> + atomic_set(_enc->frame_done_timeout_cnt, 0);
> +
> if (disp_info->intf_type == INTF_DP)
> dpu_enc->wide_bus_en =
> msm_dp_wide_bus_available(priv->dp[index]);
> else if (disp_info->intf_type == INTF_DSI)
> @@ -2115,11 +2118,12 @@ static int _dpu_encoder_status_show(struct seq_file
> *s, void *data)
> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>
> - seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d
> ",
> + seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d
> frame_done_cnt:%d",
> phys->hw_intf ? phys->hw_intf->idx - INTF_0 :
> -1,
> phys->hw_wb ? phys->hw_wb->idx - WB_0 : -1,
> atomic_read(>vsync_cnt),
> - atomic_read(>underrun_cnt));
> + atomic_read(>underrun_cnt),
> +
> atomic_read(_enc->frame_done_timeout_cnt));
>
> seq_printf(s, "mode: %s\n",
> dpu_encoder_helper_get_intf_type(phys->intf_mode));
> }
> @@ -2341,6 +2345,10 @@ static void dpu_encoder_frame_done_timeout(struct
> timer_list *t)
>
> DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
>
> + atomic_inc(_enc->frame_done_timeout_cnt);
> + if (atomic_read(_enc->frame_done_timeout_cnt) == 1)
> + msm_disp_snapshot_state(drm_enc->dev);
atomic_inc_and_test(), please
> +
> event = DPU_ENCODER_FRAME_EVENT_ERROR;
> trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
> dpu_enc->crtc_frame_event_cb(dpu_enc->crtc_frame_event_cb_data,
> event);
> @@ -2392,6 +2400,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
> *dev,
> goto fail;
>
> atomic_set(_enc->frame_done_timeout_ms, 0);
> + atomic_set(_enc->frame_done_timeout_cnt, 0);
> timer_setup(_enc->frame_done_timer,
> dpu_encoder_frame_done_timeout, 0);
>
> --
> 2.41.0
>
--
With best wishes
Dmitry
Re: [GIT PULL] exynos-drm-fixes
Hi Dave, 2023년 11월 24일 (금) 오전 10:14, Dave Airlie 님이 작성: > On Tue, 21 Nov 2023 at 09:00, Inki Dae wrote: > > > > Hi Dave and Daniel, > > > >Two fixups - fixing a potential error pointer dereference and wrong > >error checking. > Hi Inki, > > This fails to build on arm32, and it seems one of the fixes is wrong > > [airlied@dreadlord drm-fixes]$ make ARCH=arm > CROSS_COMPILE=arm-linux-gnu- O=../../arm-build-fixes/ -j16 > make[1]: Entering directory '/home/airlied/devel/kernel/arm-build-fixes' > GEN Makefile > CALL > /home/airlied/devel/kernel/dim/drm-fixes/scripts/checksyscalls.sh > CC [M] drivers/gpu/drm/exynos/exynos_drm_dma.o > > /home/airlied/devel/kernel/dim/drm-fixes/drivers/gpu/drm/exynos/exynos_drm_dma.c: > In function ‘exynos_drm_register_dma’: > > /home/airlied/devel/kernel/dim/drm-fixes/drivers/gpu/drm/exynos/exynos_drm_dma.c:119:40: > error: passing argument 1 of ‘PTR_ERR’ makes pointer from integer > without a cast [-Werror=int-conversion] > 119 | return PTR_ERR(-ENODEV); > In file included from > /home/airlied/devel/kernel/dim/drm-fixes/include/linux/string.h:9, > from > /home/airlied/devel/kernel/dim/drm-fixes/include/linux/dma-mapping.h:7, > from > /home/airlied/devel/kernel/dim/drm-fixes/include/linux/dma-map-ops.h:9, > from > > /home/airlied/devel/kernel/dim/drm-fixes/drivers/gpu/drm/exynos/exynos_drm_dma.c:7: > /home/airlied/devel/kernel/dim/drm-fixes/include/linux/err.h:49:61: > note: expected ‘const void *’ but argument is of type ‘int’ >49 | static inline long __must_check PTR_ERR(__force const void *ptr) > | ^~~ > cc1: all warnings being treated as errors > > I think it should just be return -ENODEV, since the function returns an > int. > > Please fix it up and resend. > Really sorry for this. Will resend after fixing it. Thanks, Inki Dae > Thanks, > Dave. > > > > > > >Ps. regarding the first patch, I had sent a GIT-PULL[1] but it seems > >you missed. > >[1] > https://lore.kernel.org/dri-devel/20231006040950.4397-1-inki@samsung.com/T/#u > > > >Please kindly let me know if there is any problem. > > > > Thanks, > > Inki Dae > > > > The following changes since commit > 98b1cc82c4affc16f5598d4fa14b1858671b2263: > > > > Linux 6.7-rc2 (2023-11-19 15:02:14 -0800) > > > > are available in the Git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos > tags/exynos-drm-fixes-for-v6.7-rc3 > > > > for you to fetch changes up to a30ba4bd7cdb5726d86a557c5df8df71c7bc7fad: > > > > drm/exynos: fix a wrong error checking (2023-11-21 07:41:11 +0900) > > > > > > Two fixups > > - Fix a potential error pointer dereference by checking the return value > > of exynos_drm_crtc_get_by_type() function before accessing to crtc > > object. > > - Fix a wrong error checking in exynos_drm_dma.c modules, which was > reported > > by Dan[1] > > > > [1] > https://lore.kernel.org/all/33e52277-1349-472b-a55b-ab5c3462bfcf@moroto.mountain/ > > > > > > Inki Dae (1): > > drm/exynos: fix a wrong error checking > > > > Xiang Yang (1): > > drm/exynos: fix a potential error pointer dereference > > > > drivers/gpu/drm/exynos/exynos_drm_dma.c | 8 +++- > > drivers/gpu/drm/exynos/exynos_hdmi.c| 2 ++ > > 2 files changed, 5 insertions(+), 5 deletions(-) > >
Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys_base to be relative not absolute
On Wed, Nov 22, 2023 at 02:26:55PM +0100, Andrzej Hajda wrote: > > > On 21.11.2023 13:06, Andrzej Hajda wrote: > > On 18.11.2023 00:01, Paz Zcharya wrote: > > > On Tue, Nov 14, 2023 at 10:13:59PM -0500, Rodrigo Vivi wrote: > > > > On Sun, Nov 05, 2023 at 05:27:03PM +, Paz Zcharya wrote: > > > > > > Hi Rodrigo, thanks for the great comments. > > > > > > Apologies for using a wrong/confusing terminology. I think 'phys_base' > > > is supposed to be the offset in the GEM BO, where base (or > > > "Surface Base Address") is supposed to be the GTT offset. > > > > Since base is taken from PLANE_SURF register it should be resolvable via > > GGTT to physical address pointing to actual framebuffer. > > I couldn't find anything in the specs. > > It was quite cryptic. I meant I have not found anything about assumption > from commit history that for iGPU there should be 1:1 mapping, this is why > there was an assignment "phys_base = base". Possibly the assumption is not > valid anymore for MTL(?). > Without the assumption we need to check GGTT to determine phys address. > > > The simplest approach would be then do the same as in case of DGFX: > > gen8_pte_t __iomem *gte = to_gt(i915)->ggtt->gsm; > > gen8_pte_t pte; > > > > gte += base / I915_GTT_PAGE_SIZE; > > > > pte = ioread64(gte); > > phys_base = pte & I915_GTT_PAGE_MASK; > > > > Regards > > Andrzej Hey Andrzej, Sorry for the late response. I was OOO :) I tried using the code you mentioned. It translates (in the very specific case of MTL + GOP driver) to phys_base == 0080_h. Unfortunately, it results in a corrupted screen -- the framebuffer is filled with zeros. It seems like `i915_vma_pin_ww` already reserves and binds the GEM BO to the correct address space independently of the value of `phys_base`. The only thing `phys_base` affects is the value of `stolen->start` https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/i915/gem/i915_gem_stolen.c#L747 So it seems to me that the maybe `phys_base` is named incorrectly and that it does not reflect the physical address, but the start offset of i915->mm.stolen_region. I'm happy to run more tests / debug further. Do you have more ideas of things to try? Many thanks, Paz
Re: [PATCH 1/3] drm/bridge: ti-sn65dsi86: Simplify using pm_runtime_resume_and_get()
Hi, On Thu, Nov 23, 2023 at 9:54 AM Uwe Kleine-König wrote: > > pm_runtime_resume_and_get() already drops the runtime PM usage counter > in the error case. So a call to pm_runtime_put_sync() can be dropped. > > Signed-off-by: Uwe Kleine-König > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Douglas Anderson
[PATCH] drm/msm/dpu: Capture dpu snapshot when frame_done_timer timeouts
Trigger a devcoredump to dump dpu registers and capture the drm atomic
state when the frame_done_timer timeouts.
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1cf7ff6caff4..5cf7594feb5a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -191,6 +191,7 @@ struct dpu_encoder_virt {
void *crtc_frame_event_cb_data;
atomic_t frame_done_timeout_ms;
+ atomic_t frame_done_timeout_cnt;
struct timer_list frame_done_timer;
struct msm_display_info disp_info;
@@ -1204,6 +1205,8 @@ static void dpu_encoder_virt_atomic_enable(struct
drm_encoder *drm_enc,
dpu_enc->dsc = dpu_encoder_get_dsc_config(drm_enc);
+ atomic_set(_enc->frame_done_timeout_cnt, 0);
+
if (disp_info->intf_type == INTF_DP)
dpu_enc->wide_bus_en =
msm_dp_wide_bus_available(priv->dp[index]);
else if (disp_info->intf_type == INTF_DSI)
@@ -2115,11 +2118,12 @@ static int _dpu_encoder_status_show(struct seq_file *s,
void *data)
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
- seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d",
+ seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d
frame_done_cnt:%d",
phys->hw_intf ? phys->hw_intf->idx - INTF_0 :
-1,
phys->hw_wb ? phys->hw_wb->idx - WB_0 : -1,
atomic_read(>vsync_cnt),
- atomic_read(>underrun_cnt));
+ atomic_read(>underrun_cnt),
+ atomic_read(_enc->frame_done_timeout_cnt));
seq_printf(s, "mode: %s\n",
dpu_encoder_helper_get_intf_type(phys->intf_mode));
}
@@ -2341,6 +2345,10 @@ static void dpu_encoder_frame_done_timeout(struct
timer_list *t)
DPU_ERROR_ENC(dpu_enc, "frame done timeout\n");
+ atomic_inc(_enc->frame_done_timeout_cnt);
+ if (atomic_read(_enc->frame_done_timeout_cnt) == 1)
+ msm_disp_snapshot_state(drm_enc->dev);
+
event = DPU_ENCODER_FRAME_EVENT_ERROR;
trace_dpu_enc_frame_done_timeout(DRMID(drm_enc), event);
dpu_enc->crtc_frame_event_cb(dpu_enc->crtc_frame_event_cb_data, event);
@@ -2392,6 +2400,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
*dev,
goto fail;
atomic_set(_enc->frame_done_timeout_ms, 0);
+ atomic_set(_enc->frame_done_timeout_cnt, 0);
timer_setup(_enc->frame_done_timer,
dpu_encoder_frame_done_timeout, 0);
--
2.41.0
Re: [PATCH] drm/bridge: ti-sn65dsi86: Associate PWM device to auxiliary device
Hi, On Mon, Nov 27, 2023 at 2:15 AM Uwe Kleine-König wrote: > > It's the ti_sn65dsi86.pwm auxiliary driver that creates the pwmchip, so > let the auxiliary device be the parent of the pwm device. > > Note that getting a reference to the ti-sn65dsi86's pwm using pwm_get() > isn't affected by this change as ti_sn65dsi86_add_aux_device() sets the > auxiliary device's of_node to that of the main device. > > Also change PM runtime tracking and diagnostic messages to use that one. > As the PM runtime functions also handle parent devices this should work > fine, too. > > Signed-off-by: Uwe Kleine-König > --- > Hello, > > this patch has an (easy to resolve) conflict with a patch I sent earlier > "drm/bridge: ti-sn65dsi86: Simplify using pm_runtime_resume_and_get()" > (https://lore.kernel.org/dri-devel/20231123175425.496956-2-u.kleine-koe...@pengutronix.de). > I was unsure if I should base this new patch on that older one. > > While I think the patch is fine, I'd have a better feeling about it if > someone could give feedback that the PWM still works as intended with > this change. > > Best regards > Uwe > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) This looks right to me. Reviewed-by: Douglas Anderson For now, I'll wait for Bjorn (or someone else who uses this PWM) to give a Tested-by before applying. -Doug
Re: [PATCH 0/3] Revert panel fixes and original buggy patch
On Tue, Nov 28, 2023 at 12:36:15AM +0100, Linus Walleij wrote: > On Tue, Nov 28, 2023 at 12:25 AM Laurent Pinchart wrote: > > On Tue, Nov 28, 2023 at 12:10:18AM +0100, Linus Walleij wrote: > > > This series reverts the attempts to fix the bug that went > > > into v6.7-rc1 in commit 199cf07ebd2b > > > "drm/bridge: panel: Add a device link between drm device and panel device" > > > and then it reverts that patch as well. > > > > > > Signed-off-by: Linus Walleij > > > --- > > > Linus Walleij (3): > > > Revert "driver core: Export device_is_dependent() to modules" > > > Revert "drm/bridge: panel: Check device dependency before managing > > > device link" > > > Revert "drm/bridge: panel: Add a device link between drm device and > > > panel device" > > > > To preserve bisectability, you should revert in the opposite order. > > You mean apply patch 2, then 1, then 3 so the kernel builds after each > revert? > > Yeah that's a good idea, I don't know if I should apply these though, better > someone else do it since I screwed up too much. > > Another option is to just squash the reverts into one, that bisects too :/ I thought the commits have been applied to drm-misc in a bisectable order in the first place, but that doesn't seem to be the case :-( Reverting "driver core: Export device_is_dependent() to modules" last seems to be the best option in this case. I wouldn't squash them. -- Regards, Laurent Pinchart
Re: [PATCH 0/3] Revert panel fixes and original buggy patch
On Tue, Nov 28, 2023 at 12:25 AM Laurent Pinchart wrote: > On Tue, Nov 28, 2023 at 12:10:18AM +0100, Linus Walleij wrote: > > This series reverts the attempts to fix the bug that went > > into v6.7-rc1 in commit 199cf07ebd2b > > "drm/bridge: panel: Add a device link between drm device and panel device" > > and then it reverts that patch as well. > > > > Signed-off-by: Linus Walleij > > --- > > Linus Walleij (3): > > Revert "driver core: Export device_is_dependent() to modules" > > Revert "drm/bridge: panel: Check device dependency before managing > > device link" > > Revert "drm/bridge: panel: Add a device link between drm device and > > panel device" > > To preserve bisectability, you should revert in the opposite order. You mean apply patch 2, then 1, then 3 so the kernel builds after each revert? Yeah that's a good idea, I don't know if I should apply these though, better someone else do it since I screwed up too much. Another option is to just squash the reverts into one, that bisects too :/ Yours, Linus Walleij
Re: [PATCH 0/3] Revert panel fixes and original buggy patch
Hi Linus, On Tue, Nov 28, 2023 at 12:10:18AM +0100, Linus Walleij wrote: > This series reverts the attempts to fix the bug that went > into v6.7-rc1 in commit 199cf07ebd2b > "drm/bridge: panel: Add a device link between drm device and panel device" > and then it reverts that patch as well. > > Signed-off-by: Linus Walleij > --- > Linus Walleij (3): > Revert "driver core: Export device_is_dependent() to modules" > Revert "drm/bridge: panel: Check device dependency before managing > device link" > Revert "drm/bridge: panel: Add a device link between drm device and > panel device" To preserve bisectability, you should revert in the opposite order. > > drivers/base/core.c| 1 - > drivers/gpu/drm/bridge/panel.c | 26 -- > 2 files changed, 27 deletions(-) > --- > base-commit: 95ba893c9f4feb836ddce627efd0bb6af6667031 > change-id: 20231127-revert-panel-fix-e4da3e11e7a4 -- Regards, Laurent Pinchart
Re: Radeon regression in 6.6 kernel
Alex Deucher writes: >> In that case those are the already known problems with the scheduler >> changes, aren't they? > > Yes. Those changes went into 6.7 though, not 6.6 AFAIK. Maybe I'm > misunderstanding what the original report was actually testing. If it > was 6.7, then try reverting: > 56e449603f0ac580700621a356d35d5716a62ce5 > b70438004a14f4d0f9890b3297cd66248728546c At some point it was suggested that I file a gitlab issue, but I took this to mean it was already known and being worked on. -rc3 came out today and still has the problem. Is there a known issue I could track?
Re: [PATCH] drm/msm/mdp4: flush vblank event on disable
On Tue, 28 Nov 2023 at 00:00, Abhinav Kumar wrote:
>
>
>
> On 11/27/2023 1:54 PM, Dmitry Baryshkov wrote:
> > Flush queued events when disabling the crtc. This avoids timeouts when
> > we come back and wait for dependencies (like the previous frame's
> > flip_done).
> >
> > Fixes: c8afe684c95c ("drm/msm: basic KMS driver for snapdragon")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > index 169f9de4a12a..3100957225a7 100644
> > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> > @@ -269,6 +269,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc
> > *crtc,
> > {
> > struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
> > struct mdp4_kms *mdp4_kms = get_kms(crtc);
> > + unsigned long flags;
> >
> > DBG("%s", mdp4_crtc->name);
> >
> > @@ -281,6 +282,14 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc
> > *crtc,
> > mdp_irq_unregister(_kms->base, _crtc->err);
> > mdp4_disable(mdp4_kms);
> >
> > + if (crtc->state->event && !crtc->state->active) {
> > + WARN_ON(mdp4_crtc->event);
>
> Do you need a WARN_ON() here? Just wondering how often this might happen
> and spam.
Well, it is atomic_disable(), so it happens only on modesets or output
disable. Also, this is a port of mdp5 code, see commit e765ea77b0fd
("drm/msm: Issue queued events when disabling crtc"). We don't have
reports from MDP5 code, so there should be no comparable reports for
MDP4.
> otherwise LGTM.
>
> > + spin_lock_irqsave(_kms->dev->event_lock, flags);
> > + drm_crtc_send_vblank_event(crtc, crtc->state->event);
> > + crtc->state->event = NULL;
> > + spin_unlock_irqrestore(_kms->dev->event_lock, flags);
> > + }
> > +
> > mdp4_crtc->enabled = false;
> > }
> >
--
With best wishes
Dmitry
[PATCH 1/3] Revert "driver core: Export device_is_dependent() to modules"
This reverts commit 1d5e8f4bf06da86b71cc9169110d1a0e1e7af337. Greg says: "why exactly is this needed? Nothing outside of the driver core should be needing this function, it shouldn't be public at all (I missed that before.) So please, revert it for now, let's figure out why DRM thinks this is needed for it's devices, and yet no other bus/subsystem does." Link: https://lore.kernel.org/dri-devel/2023112739-willing-sighing-6bdd@gregkh/ Signed-off-by: Linus Walleij --- drivers/base/core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index bfd2bf0364b7..67ba592afc77 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -328,7 +328,6 @@ int device_is_dependent(struct device *dev, void *target) } return ret; } -EXPORT_SYMBOL_GPL(device_is_dependent); static void device_link_init_status(struct device_link *link, struct device *consumer, -- 2.41.0
[PATCH 2/3] Revert "drm/bridge: panel: Check device dependency before managing device link"
This reverts commit 39d5b6a64ace77d0c11c398d272218df5f939abb.
This patch was causing build errors by using an unexported
function from the device core, which Greg questions the
saneness in exporting.
Link:
https://lore.kernel.org/lkml/cacrpkdagzxd6hbix7mvunjajtmepg00pp6+nj1p0jrfj-ar...@mail.gmail.com/T/
Signed-off-by: Linus Walleij
---
drivers/gpu/drm/bridge/panel.c | 27 +--
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index 5e8980023407..e48823a4f1ed 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -23,7 +23,6 @@ struct panel_bridge {
struct drm_panel *panel;
struct device_link *link;
u32 connector_type;
- bool is_independent;
};
static inline struct panel_bridge *
@@ -68,17 +67,12 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
struct drm_device *drm_dev = bridge->dev;
int ret;
- panel_bridge->is_independent = !device_is_dependent(drm_dev->dev,
- panel->dev);
-
- if (panel_bridge->is_independent) {
- panel_bridge->link = device_link_add(drm_dev->dev, panel->dev,
-DL_FLAG_STATELESS);
- if (!panel_bridge->link) {
- DRM_ERROR("Failed to add device link between %s and
%s\n",
- dev_name(drm_dev->dev), dev_name(panel->dev));
- return -EINVAL;
- }
+ panel_bridge->link = device_link_add(drm_dev->dev, panel->dev,
+DL_FLAG_STATELESS);
+ if (!panel_bridge->link) {
+ DRM_ERROR("Failed to add device link between %s and %s\n",
+ dev_name(drm_dev->dev), dev_name(panel->dev));
+ return -EINVAL;
}
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
@@ -86,8 +80,7 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
if (!bridge->encoder) {
DRM_ERROR("Missing encoder\n");
- if (panel_bridge->is_independent)
- device_link_del(panel_bridge->link);
+ device_link_del(panel_bridge->link);
return -ENODEV;
}
@@ -99,8 +92,7 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
panel_bridge->connector_type);
if (ret) {
DRM_ERROR("Failed to initialize connector\n");
- if (panel_bridge->is_independent)
- device_link_del(panel_bridge->link);
+ device_link_del(panel_bridge->link);
return ret;
}
@@ -123,8 +115,7 @@ static void panel_bridge_detach(struct drm_bridge *bridge)
struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
struct drm_connector *connector = _bridge->connector;
- if (panel_bridge->is_independent)
- device_link_del(panel_bridge->link);
+ device_link_del(panel_bridge->link);
/*
* Cleanup the connector if we know it was initialized.
--
2.41.0
[PATCH 3/3] Revert "drm/bridge: panel: Add a device link between drm device and panel device"
This reverts commit 199cf07ebd2b0d41185ac79b895547d45610b681.
This patch creates bugs on devices where the DRM device is
the ancestor of the panel devices.
Attempts to fix this have failed because it leads to using
device core functionality which is questionable.
Reported-by: Linus Walleij
Link:
https://lore.kernel.org/lkml/cacrpkdagzxd6hbix7mvunjajtmepg00pp6+nj1p0jrfj-ar...@mail.gmail.com/T/
Signed-off-by: Linus Walleij
---
drivers/gpu/drm/bridge/panel.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index e48823a4f1ed..7f41525f7a6e 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -4,8 +4,6 @@
* Copyright (C) 2017 Broadcom
*/
-#include
-
#include
#include
#include
@@ -21,7 +19,6 @@ struct panel_bridge {
struct drm_bridge bridge;
struct drm_connector connector;
struct drm_panel *panel;
- struct device_link *link;
u32 connector_type;
};
@@ -63,24 +60,13 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
{
struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
struct drm_connector *connector = _bridge->connector;
- struct drm_panel *panel = panel_bridge->panel;
- struct drm_device *drm_dev = bridge->dev;
int ret;
- panel_bridge->link = device_link_add(drm_dev->dev, panel->dev,
-DL_FLAG_STATELESS);
- if (!panel_bridge->link) {
- DRM_ERROR("Failed to add device link between %s and %s\n",
- dev_name(drm_dev->dev), dev_name(panel->dev));
- return -EINVAL;
- }
-
if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
return 0;
if (!bridge->encoder) {
DRM_ERROR("Missing encoder\n");
- device_link_del(panel_bridge->link);
return -ENODEV;
}
@@ -92,7 +78,6 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
panel_bridge->connector_type);
if (ret) {
DRM_ERROR("Failed to initialize connector\n");
- device_link_del(panel_bridge->link);
return ret;
}
@@ -115,8 +100,6 @@ static void panel_bridge_detach(struct drm_bridge *bridge)
struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
struct drm_connector *connector = _bridge->connector;
- device_link_del(panel_bridge->link);
-
/*
* Cleanup the connector if we know it was initialized.
*
--
2.41.0
[PATCH 0/3] Revert panel fixes and original buggy patch
This series reverts the attempts to fix the bug that went into v6.7-rc1 in commit 199cf07ebd2b "drm/bridge: panel: Add a device link between drm device and panel device" and then it reverts that patch as well. Signed-off-by: Linus Walleij --- Linus Walleij (3): Revert "driver core: Export device_is_dependent() to modules" Revert "drm/bridge: panel: Check device dependency before managing device link" Revert "drm/bridge: panel: Add a device link between drm device and panel device" drivers/base/core.c| 1 - drivers/gpu/drm/bridge/panel.c | 26 -- 2 files changed, 27 deletions(-) --- base-commit: 95ba893c9f4feb836ddce627efd0bb6af6667031 change-id: 20231127-revert-panel-fix-e4da3e11e7a4 Best regards, -- Linus Walleij
Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules
On Mon, Nov 27, 2023 at 7:20 PM Greg KH wrote: [Maxime] > > So, a committer just applied this to drm-misc-fixes without your > > approval. That was me, mea culpa. I learned a lesson. Or two. > Wait, why exactly is this needed? Nothing outside of the driver core > should be needing this function, it shouldn't be public at all (I missed > that before.) > > So please, revert it for now, let's figure out why DRM thinks this is > needed for it's devices, and yet no other bus/subsystem does. I'm preparing a revert series back to the original patch causing the issue so we can clear the original bug out of the way, take a step back and fix this properly. Yours, Linus Walleij
Re: [PATCH v2] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer
Applied. Thanks!
Alex
On Thu, Nov 23, 2023 at 3:22 AM Christian König
wrote:
>
> Am 23.11.23 um 02:22 schrieb Lu Yao:
> > For 'AMDGPU_FAMILY_SI' family cards, in 'si_common_early_init' func, init
> > 'didt_rreg' and 'didt_wreg' to 'NULL'. But in func
> > 'amdgpu_debugfs_regs_didt_read/write', using 'RREG32_DIDT' 'WREG32_DIDT'
> > lacks of relevant judgment. And other 'amdgpu_ip_block_version' that use
> > these two definitions won't be added for 'AMDGPU_FAMILY_SI'.
> >
> > So, add null pointer judgment before calling.
> >
> > Signed-off-by: Lu Yao
>
> Reviewed-by: Christian König
>
> > ---
> > Changes in v2:
> >1. Drop dev_err message.
> >2. Change error code from 'EPERM' to 'EOPNOTSUPP'
> > Link to v1:
> > https://lore.kernel.org/all/20231122093509.34302-1-ya...@kylinos.cn/
> > Thanks Christian for his comments.
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > index a53f436fa9f1..e098cd66fa2a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> > @@ -638,6 +638,9 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct
> > file *f, char __user *buf,
> > if (size & 0x3 || *pos & 0x3)
> > return -EINVAL;
> >
> > + if (adev->didt_rreg == NULL)
> > + return -EOPNOTSUPP;
> > +
> > r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> > if (r < 0) {
> > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > @@ -694,6 +697,9 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct
> > file *f, const char __user
> > if (size & 0x3 || *pos & 0x3)
> > return -EINVAL;
> >
> > + if (adev->didt_wreg == NULL)
> > + return -EOPNOTSUPP;
> > +
> > r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
> > if (r < 0) {
> > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
>
Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link
On Mon, Nov 27, 2023 at 5:29 PM Maxime Ripard wrote: > On Mon, Nov 27, 2023 at 05:03:53PM +0100, Linus Walleij wrote: > > > Liu Ying (2): > > > driver core: Export device_is_dependent() to modules > > > drm/bridge: panel: Check device dependency before managing device link > > > > I just applied patch 1 directly to the drm-misc-fixes so we don't have to > > revert and then re-apply patches, because that is a bigger evil. (We can't > > rebase these branches...) > > Erm, you did wait for GKH or Rafael's ACK to do that, right? No. It is a bigger evil to leave the tree broken than to enforce formal process, and it is pretty self-evident. If any of them get annoyed about it we can revert the patch, or both. Yours, Linus Walleij
Re: [PATCH] drm/msm/mdp4: flush vblank event on disable
On 11/27/2023 1:54 PM, Dmitry Baryshkov wrote:
Flush queued events when disabling the crtc. This avoids timeouts when
we come back and wait for dependencies (like the previous frame's
flip_done).
Fixes: c8afe684c95c ("drm/msm: basic KMS driver for snapdragon")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 169f9de4a12a..3100957225a7 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -269,6 +269,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
{
struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
struct mdp4_kms *mdp4_kms = get_kms(crtc);
+ unsigned long flags;
DBG("%s", mdp4_crtc->name);
@@ -281,6 +282,14 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
mdp_irq_unregister(_kms->base, _crtc->err);
mdp4_disable(mdp4_kms);
+ if (crtc->state->event && !crtc->state->active) {
+ WARN_ON(mdp4_crtc->event);
Do you need a WARN_ON() here? Just wondering how often this might happen
and spam.
otherwise LGTM.
+ spin_lock_irqsave(_kms->dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(_kms->dev->event_lock, flags);
+ }
+
mdp4_crtc->enabled = false;
}
[PATCH] drm/msm/mdp4: flush vblank event on disable
Flush queued events when disabling the crtc. This avoids timeouts when
we come back and wait for dependencies (like the previous frame's
flip_done).
Fixes: c8afe684c95c ("drm/msm: basic KMS driver for snapdragon")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 169f9de4a12a..3100957225a7 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -269,6 +269,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
{
struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
struct mdp4_kms *mdp4_kms = get_kms(crtc);
+ unsigned long flags;
DBG("%s", mdp4_crtc->name);
@@ -281,6 +282,14 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
mdp_irq_unregister(_kms->base, _crtc->err);
mdp4_disable(mdp4_kms);
+ if (crtc->state->event && !crtc->state->active) {
+ WARN_ON(mdp4_crtc->event);
+ spin_lock_irqsave(_kms->dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(_kms->dev->event_lock, flags);
+ }
+
mdp4_crtc->enabled = false;
}
--
2.39.2
Re: [PATCH v6 2/2] drm/i915/guc: Close deregister-context race against CT-loss
On Tue, Nov 14, 2023 at 07:52:29AM -0800, Alan Previn wrote:
> If we are at the end of suspend or very early in resume
> its possible an async fence signal (via rcu_call) is triggered
> to free_engines which could lead us to the execution of
> the context destruction worker (after a prior worker flush).
>
> Thus, when suspending, insert rcu_barriers at the start
> of i915_gem_suspend (part of driver's suspend prepare) and
> again in i915_gem_suspend_late so that all such cases have
> completed and context destruction list isn't missing anything.
>
> In destroyed_worker_func, close the race against CT-loss
> by checking that CT is enabled before calling into
> deregister_destroyed_contexts.
>
> Based on testing, guc_lrc_desc_unpin may still race and fail
> as we traverse the GuC's context-destroy list because the
> CT could be disabled right before calling GuC's CT send function.
>
> We've witnessed this race condition once every ~6000-8000
> suspend-resume cycles while ensuring workloads that render
> something onscreen is continuously started just before
> we suspend (and the workload is small enough to complete
> and trigger the queued engine/context free-up either very
> late in suspend or very early in resume).
>
> In such a case, we need to unroll the entire process because
> guc-lrc-unpin takes a gt wakeref which only gets released in
> the G2H IRQ reply that never comes through in this corner
> case. Without the unroll, the taken wakeref is leaked and will
> cascade into a kernel hang later at the tail end of suspend in
> this function:
>
>intel_wakeref_wait_for_idle(>wakeref)
>(called by) - intel_gt_pm_wait_for_idle
>(called by) - wait_for_suspend
>
> Thus, do an unroll in guc_lrc_desc_unpin and deregister_destroyed_-
> contexts if guc_lrc_desc_unpin fails due to CT send falure.
> When unrolling, keep the context in the GuC's destroy-list so
> it can get picked up on the next destroy worker invocation
> (if suspend aborted) or get fully purged as part of a GuC
> sanitization (end of suspend) or a reset flow.
>
> Signed-off-by: Alan Previn
> Signed-off-by: Anshuman Gupta
> Tested-by: Mousumi Jana
> ---
> drivers/gpu/drm/i915/gem/i915_gem_pm.c| 10 +++
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 81 ---
> 2 files changed, 80 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> index 0d812f4d787d..3b27218aabe2 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> @@ -28,6 +28,13 @@ void i915_gem_suspend(struct drm_i915_private *i915)
> GEM_TRACE("%s\n", dev_name(i915->drm.dev));
>
> intel_wakeref_auto(>runtime_pm.userfault_wakeref, 0);
> + /*
> + * On rare occasions, we've observed the fence completion triggers
> + * free_engines asynchronously via rcu_call. Ensure those are done.
> + * This path is only called on suspend, so it's an acceptable cost.
> + */
> + rcu_barrier();
> +
> flush_workqueue(i915->wq);
>
> /*
> @@ -160,6 +167,9 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
>* machine in an unusable condition.
>*/
>
> + /* Like i915_gem_suspend, flush tasks staged from fence triggers */
> + rcu_barrier();
> +
> for_each_gt(gt, i915, i)
> intel_gt_suspend_late(gt);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 9d1915482898..225747115f78 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -236,6 +236,13 @@ set_context_destroyed(struct intel_context *ce)
> ce->guc_state.sched_state |= SCHED_STATE_DESTROYED;
> }
>
> +static inline void
> +clr_context_destroyed(struct intel_context *ce)
> +{
> + lockdep_assert_held(>guc_state.lock);
> + ce->guc_state.sched_state &= ~SCHED_STATE_DESTROYED;
> +}
> +
> static inline bool context_pending_disable(struct intel_context *ce)
> {
> return ce->guc_state.sched_state & SCHED_STATE_PENDING_DISABLE;
> @@ -613,6 +620,8 @@ static int guc_submission_send_busy_loop(struct intel_guc
> *guc,
>u32 g2h_len_dw,
>bool loop)
> {
> + int ret;
> +
> /*
>* We always loop when a send requires a reply (i.e. g2h_len_dw > 0),
>* so we don't handle the case where we don't get a reply because we
> @@ -623,7 +632,11 @@ static int guc_submission_send_busy_loop(struct
> intel_guc *guc,
> if (g2h_len_dw)
> atomic_inc(>outstanding_submission_g2h);
>
> - return intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
> + ret = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
> + if (ret)
> + atomic_dec(>outstanding_submission_g2h);
> +
> +
Re: [Nouveau] [PATCH 3/3] nouveau/gsp: add some basic registry entries.
On Tue, 28 Nov 2023 at 06:48, Timur Tabi wrote:
>
> On Tue, Oct 31, 2023 at 12:20 AM Dave Airlie wrote:
> > rpc->size = sizeof(*rpc);
> > - rpc->numEntries = 1;
> > - rpc->entries[0].nameOffset = offsetof(typeof(*rpc), entries[1]);
> > - rpc->entries[0].type = 1;
> > - rpc->entries[0].data = 0;
> > - rpc->entries[0].length = 4;
> > -
> > - strings = (char *)>entries[1];
> > - strings[0] = '\0';
> > + rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
> > +
> > + str_offset = offsetof(typeof(*rpc),
> > entries[NV_GSP_REG_NUM_ENTRIES]);
> > + strings = (char *)>entries[NV_GSP_REG_NUM_ENTRIES];
> > + for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> > + int name_len = strlen(r535_registry_entries[i].name) + 1;
> > + rpc->entries[i].nameOffset = str_offset;
> > + rpc->entries[i].type = 1;
> > + rpc->entries[i].data = r535_registry_entries[i].value;
> > + rpc->entries[i].length = 4;
> > + memcpy(strings, r535_registry_entries[i].name, name_len);
> > + strings += name_len;
> > + str_offset += name_len;
> > + }
>
> I'm working on a patch that replaces this code with a
> dynamically-generated registry so that we can set registry keys via
> the driver's command-line (like the Nvidia driver).
I'm not sure we'd want that, except maybe as a debugging aid, I'd
really like to have nouveau just pick the correct set of registry
entries, but I suppose there might be some cases where setting the
from the command line would be good for testing.
> a bug here. rpc->size must be the total size of the RPC, including
> all the PACKED_REGISTRY_ENTRY structs and the strings that follow
> them. You can see this by looking at RmPackageRegistry() and
> regCountEntriesAndSize() in OpenRM.
Oh interesting, I'll take a look today.
Dave.
[PATCH] drm: lcdif: Switch to drmm_mode_config_init
Switch from deprecated unmanaged drm_mode_config_init() to
managed drmm_mode_config_init(). No functional change.
Signed-off-by: Marek Vasut
---
Cc: Daniel Vetter
Cc: David Airlie
Cc: Fabio Estevam
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: NXP Linux Team
Cc: Pengutronix Kernel Team
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: Stefan Agner
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-ker...@lists.infradead.org
---
drivers/gpu/drm/mxsfb/lcdif_drv.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c
b/drivers/gpu/drm/mxsfb/lcdif_drv.c
index 18de2f17e2491..ea10bf81582e9 100644
--- a/drivers/gpu/drm/mxsfb/lcdif_drv.c
+++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c
@@ -167,7 +167,11 @@ static int lcdif_load(struct drm_device *drm)
return ret;
/* Modeset init */
- drm_mode_config_init(drm);
+ ret = drmm_mode_config_init(drm);
+ if (ret) {
+ dev_err(drm->dev, "Failed to initialize mode config\n");
+ return ret;
+ }
ret = lcdif_kms_init(lcdif);
if (ret < 0) {
@@ -227,7 +231,6 @@ static void lcdif_unload(struct drm_device *drm)
drm_crtc_vblank_off(>crtc);
drm_kms_helper_poll_fini(drm);
- drm_mode_config_cleanup(drm);
pm_runtime_put_sync(drm->dev);
pm_runtime_disable(drm->dev);
--
2.42.0
[PATCH v2] drm/mxsfb: Switch to drmm_mode_config_init
Switch from deprecated unmanaged drm_mode_config_init() to
managed drmm_mode_config_init(). No functional change.
Signed-off-by: Marek Vasut
---
Cc: Daniel Vetter
Cc: David Airlie
Cc: Fabio Estevam
Cc: Maarten Lankhorst
Cc: Maxime Ripard
Cc: NXP Linux Team
Cc: Pengutronix Kernel Team
Cc: Sascha Hauer
Cc: Shawn Guo
Cc: Stefan Agner
Cc: Thomas Zimmermann
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-ker...@lists.infradead.org
---
V2: Drop matching drm_mode_config_cleanup
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 3bfa369b2507e..cfa3176ebe2c2 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -248,7 +248,11 @@ static int mxsfb_load(struct drm_device *drm,
pm_runtime_enable(drm->dev);
/* Modeset init */
- drm_mode_config_init(drm);
+ ret = drmm_mode_config_init(drm);
+ if (ret) {
+ dev_err(drm->dev, "Failed to initialize mode config\n");
+ goto err_vblank;
+ }
ret = mxsfb_kms_init(mxsfb);
if (ret < 0) {
@@ -311,7 +315,6 @@ static int mxsfb_load(struct drm_device *drm,
static void mxsfb_unload(struct drm_device *drm)
{
drm_kms_helper_poll_fini(drm);
- drm_mode_config_cleanup(drm);
pm_runtime_get_sync(drm->dev);
mxsfb_irq_uninstall(drm);
--
2.42.0
Re: [PATCH 14/14] drm: Remove Kconfig option for legacy support (CONFIG_DRM_LEGACY)
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote: > > Remove CONFIG_DRM_LEGACY from Kconfig. Nothing depends on the option. > > Signed-off-by: Thomas Zimmermann Acked-by: Alex Deucher > --- > drivers/gpu/drm/Kconfig | 21 - > 1 file changed, 21 deletions(-) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index cdbc56e076498..5150c8699a8b5 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -407,27 +407,6 @@ config DRM_HYPERV > > If M is selected the module will be called hyperv_drm. > > -# Keep legacy drivers last > - > -menuconfig DRM_LEGACY > - bool "Enable legacy drivers (DANGEROUS)" > - depends on DRM && MMU > - help > - Enable legacy DRI1 drivers. Those drivers expose unsafe and > dangerous > - APIs to user-space, which can be used to circumvent access > - restrictions and other security measures. For backwards > compatibility > - those drivers are still available, but their use is highly > - inadvisable and might harm your system. > - > - You are recommended to use the safe modeset-only drivers instead, > and > - perform 3D emulation in user-space. > - > - Unless you have strong reasons to go rogue, say "N". > - > -if DRM_LEGACY > -# leave here to list legacy drivers > -endif # DRM_LEGACY > - > config DRM_EXPORT_FOR_TESTS > bool > > -- > 2.42.1 >
Re: [PATCH 13/14] char/agp: Remove frontend code
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> The AGP subsystem supports a user-space interface via /dev/agpgart. It
> is only enabled with DRM support for mode setting in user space. (i.e.,
> CONFIG_DRM_LEGACY). All of that DRM code has been removed and the option
> will go away. Hence remove the AGP frontend.
>
> Modern DRM drivers with kernel mode setting handle AGP support internally.
>
> Signed-off-by: Thomas Zimmermann
Acked-by: Alex Deucher
> ---
> drivers/char/agp/Makefile |6 -
> drivers/char/agp/agp.h |9 -
> drivers/char/agp/backend.c | 11 -
> drivers/char/agp/compat_ioctl.c | 291 -
> drivers/char/agp/compat_ioctl.h | 106 ---
> drivers/char/agp/frontend.c | 1068 ---
> 6 files changed, 1491 deletions(-)
> delete mode 100644 drivers/char/agp/compat_ioctl.c
> delete mode 100644 drivers/char/agp/compat_ioctl.h
> delete mode 100644 drivers/char/agp/frontend.c
>
> diff --git a/drivers/char/agp/Makefile b/drivers/char/agp/Makefile
> index 25834557e4865..43b09cf193bb7 100644
> --- a/drivers/char/agp/Makefile
> +++ b/drivers/char/agp/Makefile
> @@ -1,12 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> agpgart-y := backend.o generic.o isoch.o
>
> -ifeq ($(CONFIG_DRM_LEGACY),y)
> -agpgart-$(CONFIG_COMPAT) += compat_ioctl.o
> -agpgart-y += frontend.o
> -endif
> -
> -
> obj-$(CONFIG_AGP) += agpgart.o
> obj-$(CONFIG_AGP_ALI) += ali-agp.o
> obj-$(CONFIG_AGP_ATI) += ati-agp.o
> diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
> index 8771dcc9b8e2f..5c36ab85f80b7 100644
> --- a/drivers/char/agp/agp.h
> +++ b/drivers/char/agp/agp.h
> @@ -185,15 +185,6 @@ void agp_put_bridge(struct agp_bridge_data *bridge);
> int agp_add_bridge(struct agp_bridge_data *bridge);
> void agp_remove_bridge(struct agp_bridge_data *bridge);
>
> -/* Frontend routines. */
> -#if IS_ENABLED(CONFIG_DRM_LEGACY)
> -int agp_frontend_initialize(void);
> -void agp_frontend_cleanup(void);
> -#else
> -static inline int agp_frontend_initialize(void) { return 0; }
> -static inline void agp_frontend_cleanup(void) {}
> -#endif
> -
> /* Generic routines. */
> void agp_generic_enable(struct agp_bridge_data *bridge, u32 mode);
> int agp_generic_create_gatt_table(struct agp_bridge_data *bridge);
> diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
> index 0e19c600db531..1776afd3ee078 100644
> --- a/drivers/char/agp/backend.c
> +++ b/drivers/char/agp/backend.c
> @@ -293,13 +293,6 @@ int agp_add_bridge(struct agp_bridge_data *bridge)
> }
>
> if (list_empty(_bridges)) {
> - error = agp_frontend_initialize();
> - if (error) {
> - dev_info(>dev->dev,
> -"agp_frontend_initialize() failed\n");
> - goto frontend_err;
> - }
> -
> dev_info(>dev->dev, "AGP aperture is %dM @ 0x%lx\n",
> bridge->driver->fetch_size(), bridge->gart_bus_addr);
>
> @@ -308,8 +301,6 @@ int agp_add_bridge(struct agp_bridge_data *bridge)
> list_add(>list, _bridges);
> return 0;
>
> -frontend_err:
> - agp_backend_cleanup(bridge);
> err_out:
> module_put(bridge->driver->owner);
> err_put_bridge:
> @@ -323,8 +314,6 @@ void agp_remove_bridge(struct agp_bridge_data *bridge)
> {
> agp_backend_cleanup(bridge);
> list_del(>list);
> - if (list_empty(_bridges))
> - agp_frontend_cleanup();
> module_put(bridge->driver->owner);
> }
> EXPORT_SYMBOL_GPL(agp_remove_bridge);
> diff --git a/drivers/char/agp/compat_ioctl.c b/drivers/char/agp/compat_ioctl.c
> deleted file mode 100644
> index 52ffe1706ce05..0
> --- a/drivers/char/agp/compat_ioctl.c
> +++ /dev/null
> @@ -1,291 +0,0 @@
> -/*
> - * AGPGART driver frontend compatibility ioctls
> - * Copyright (C) 2004 Silicon Graphics, Inc.
> - * Copyright (C) 2002-2003 Dave Jones
> - * Copyright (C) 1999 Jeff Hartmann
> - * Copyright (C) 1999 Precision Insight, Inc.
> - * Copyright (C) 1999 Xi Graphics, Inc.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included
> - * in all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> - * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR
Re: [PATCH v2] Remove custom dumb_map_offset implementation in msm driver
On Mon, 27 Nov 2023 at 22:52, Rob Clark wrote:
>
> On Tue, Nov 21, 2023 at 5:14 AM Dmitry Baryshkov
> wrote:
> >
> > On Tue, 21 Nov 2023 at 04:26, Rob Clark wrote:
> > >
> > > On Wed, Nov 15, 2023 at 11:33 AM Dmitry Baryshkov
> > > wrote:
> > > >
> > > > On Wed, 15 Nov 2023 at 20:46, Dipam Turkar wrote:
> > > > >
> > > > > They are not outdated, my bad. I went through the locks' code and saw
> > > > > that they have been updated. But they are probably not necessary here
> > > > > as most of the drivers do not use any form of locking in their
> > > > > implementations. The generic implementations
> > > > > drm_gem_dumb_map_offset() and drm_gem_ttm_dumb_map_offset() do not
> > > > > have any locking mechanisms either.
> > > >
> > > > Excuse me, but this doesn't sound right to me. There are different
> > > > drivers with different implementations. So either we'd need a good
> > > > explanation of why it is not necessary, or this patch is NAKed.
> > >
> > > Digging a bit thru history, it looks like commit 0de23977cfeb
> > > ("drm/gem: convert to new unified vma manager") made external locking
> > > unnecessary, since the vma mgr already had it's own internal locking.
> >
> > So, should we drop our own locking system?
>
> specifically for _just_ vma_offset_manager/vma_node, we could. But I
> think that only amounts to mmap_offset().
I see. I'll try digging into the mentioned commit. In the meantime,
this looks like an R-B from you, doesn't it?
>
> BR,
> -R
>
> > >
> > > BR,
> > > -R
> > >
> > > > >
> > > > > Thanks and regards
> > > > > Dipam Turkar
> > > > >
> > > > > On Wed, Nov 15, 2023 at 8:37 PM Dmitry Baryshkov
> > > > > wrote:
> > > > >>
> > > > >> On Wed, 15 Nov 2023 at 16:30, Dipam Turkar
> > > > >> wrote:
> > > > >> >
> > > > >> > Make msm use drm_gem_create_map_offset() instead of its custom
> > > > >> > implementation for associating GEM object with a fake offset.
> > > > >> > Since,
> > > > >> > we already have this generic implementation, we don't need the
> > > > >> > custom
> > > > >> > implementation and it is better to standardize the code for GEM
> > > > >> > based
> > > > >> > drivers. This also removes the outdated locking leftovers.
> > > > >>
> > > > >> Why are they outdated?
> > > > >>
> > > > >> >
> > > > >> > Signed-off-by: Dipam Turkar
> > > > >> > ---
> > > > >> > drivers/gpu/drm/msm/msm_drv.c | 2 +-
> > > > >> > drivers/gpu/drm/msm/msm_gem.c | 21 -
> > > > >> > drivers/gpu/drm/msm/msm_gem.h | 2 --
> > > > >> > 3 files changed, 1 insertion(+), 24 deletions(-)
> > > > >> >
> > > > >> > Changes in v2:
> > > > >> > Modify commit message to include the absence of internal locking
> > > > >> > leftovers
> > > > >> > around allocating a fake offset in msm_gem_mmap_offset() in the
> > > > >> > generic
> > > > >> > implementation drm_gem_create_map_offset().
> > > > >> >
> > > > >> > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> > > > >> > b/drivers/gpu/drm/msm/msm_drv.c
> > > > >> > index a428951ee539..86a15992c717 100644
> > > > >> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > > >> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > > >> > @@ -1085,7 +1085,7 @@ static const struct drm_driver msm_driver = {
> > > > >> > .open = msm_open,
> > > > >> > .postclose = msm_postclose,
> > > > >> > .dumb_create= msm_gem_dumb_create,
> > > > >> > - .dumb_map_offset= msm_gem_dumb_map_offset,
> > > > >> > + .dumb_map_offset= drm_gem_dumb_map_offset,
> > > > >> > .gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
> > > > >> > #ifdef CONFIG_DEBUG_FS
> > > > >> > .debugfs_init = msm_debugfs_init,
> > > > >> > diff --git a/drivers/gpu/drm/msm/msm_gem.c
> > > > >> > b/drivers/gpu/drm/msm/msm_gem.c
> > > > >> > index db1e748daa75..489694ef79cb 100644
> > > > >> > --- a/drivers/gpu/drm/msm/msm_gem.c
> > > > >> > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > > > >> > @@ -671,27 +671,6 @@ int msm_gem_dumb_create(struct drm_file
> > > > >> > *file, struct drm_device *dev,
> > > > >> > MSM_BO_SCANOUT | MSM_BO_WC, >handle,
> > > > >> > "dumb");
> > > > >> > }
> > > > >> >
> > > > >> > -int msm_gem_dumb_map_offset(struct drm_file *file, struct
> > > > >> > drm_device *dev,
> > > > >> > - uint32_t handle, uint64_t *offset)
> > > > >> > -{
> > > > >> > - struct drm_gem_object *obj;
> > > > >> > - int ret = 0;
> > > > >> > -
> > > > >> > - /* GEM does all our handle to object mapping */
> > > > >> > - obj = drm_gem_object_lookup(file, handle);
> > > > >> > - if (obj == NULL) {
> > > > >> > - ret = -ENOENT;
> > > > >> > - goto fail;
> > > > >> > - }
> > > > >> > -
> > > > >> > - *offset = msm_gem_mmap_offset(obj);
> > > > >> > -
> > > > >> > - drm_gem_object_put(obj);
> > > > >> > -
> > > > >> > -fail:
> > > > >> > - return ret;
> > > > >> > -}
> > > > >> >
Re: [PATCH 12/14] drm: Remove source code for non-KMS drivers
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote: > > Remove all remaining source code for non-KMS drivers. These drivers > have been removed in v6.3 and won't comeback. > > Signed-off-by: Thomas Zimmermann Acked-by: Alex Deucher
Re: [PATCH 11/14] drm: Remove locking for legacy ioctls and DRM_UNLOCKED
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> Modern DRM drivers acquire ioctl locks by themselves. Legacy ioctls
> for user-space mode setting used to acquire drm_global_mutex. After
> removing the ioctl entry points, also remove the locking code. The only
> legacy ioctl without global locking was VBLANK_WAIT, which has been
> removed as well. Hence remove the related DRM_UNLOCKED flag.
>
> Signed-off-by: Thomas Zimmermann
Acked-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_ioc32.c | 2 +-
> drivers/gpu/drm/drm_ioctl.c | 23 +++
> include/drm/drm_ioctl.h | 11 ---
> 3 files changed, 8 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index 910cadf14756e..129e2b91dbfe7 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -273,7 +273,7 @@ static int compat_drm_wait_vblank(struct file *file,
> unsigned int cmd,
> req.request.type = req32.request.type;
> req.request.sequence = req32.request.sequence;
> req.request.signal = req32.request.signal;
> - err = drm_ioctl_kernel(file, drm_wait_vblank_ioctl, ,
> DRM_UNLOCKED);
> + err = drm_ioctl_kernel(file, drm_wait_vblank_ioctl, , 0);
>
> req32.reply.type = req.reply.type;
> req32.reply.sequence = req.reply.sequence;
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 9c6326b908e74..1cf1de342d6aa 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -583,7 +583,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>
> DRM_IOCTL_DEF(DRM_IOCTL_FINISH, drm_noop, DRM_AUTH),
>
> - DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank_ioctl,
> DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank_ioctl, 0),
>
> DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
> @@ -716,7 +716,7 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t
> *func, void *kdata,
> {
> struct drm_file *file_priv = file->private_data;
> struct drm_device *dev = file_priv->minor->dev;
> - int retcode;
> + int ret;
>
> /* Update drm_file owner if fd was passed along. */
> drm_file_update_pid(file_priv);
> @@ -724,20 +724,11 @@ long drm_ioctl_kernel(struct file *file, drm_ioctl_t
> *func, void *kdata,
> if (drm_dev_is_unplugged(dev))
> return -ENODEV;
>
> - retcode = drm_ioctl_permit(flags, file_priv);
> - if (unlikely(retcode))
> - return retcode;
> -
> - /* Enforce sane locking for modern driver ioctls. */
> - if (likely(!drm_core_check_feature(dev, DRIVER_LEGACY)) ||
> - (flags & DRM_UNLOCKED))
> - retcode = func(dev, kdata, file_priv);
> - else {
> - mutex_lock(_global_mutex);
> - retcode = func(dev, kdata, file_priv);
> - mutex_unlock(_global_mutex);
> - }
> - return retcode;
> + ret = drm_ioctl_permit(flags, file_priv);
> + if (unlikely(ret))
> + return ret;
> +
> + return func(dev, kdata, file_priv);
> }
> EXPORT_SYMBOL(drm_ioctl_kernel);
>
> diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> index 6ed61c371f6ce..171760b6c4a14 100644
> --- a/include/drm/drm_ioctl.h
> +++ b/include/drm/drm_ioctl.h
> @@ -109,17 +109,6 @@ enum drm_ioctl_flags {
> * This is equivalent to callers with the SYSADMIN capability.
> */
> DRM_ROOT_ONLY = BIT(2),
> - /**
> -* @DRM_UNLOCKED:
> -*
> -* Whether _ioctl_desc.func should be called with the DRM BKL held
> -* or not. Enforced as the default for all modern drivers, hence there
> -* should never be a need to set this flag.
> -*
> -* Do not use anywhere else than for the VBLANK_WAIT IOCTL, which is
> the
> -* only legacy IOCTL which needs this.
> -*/
> - DRM_UNLOCKED= BIT(4),
> /**
> * @DRM_RENDER_ALLOW:
> *
> --
> 2.42.1
>
Re: [PATCH 10/14] drm: Remove support for legacy drivers
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> Remove all hooks and calls into code for user-space mode setting from
> the DRM core. Without the drivers and ioctl entry points, none of this
> is required any longer.
>
> Signed-off-by: Thomas Zimmermann
Acked-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_auth.c | 8 +
> drivers/gpu/drm/drm_drv.c| 17 --
> drivers/gpu/drm/drm_file.c | 64 ++--
> drivers/gpu/drm/drm_vblank.c | 19 ---
> 4 files changed, 3 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 2ed2585ded378..252c105d614ff 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -37,13 +37,12 @@
> #include
>
> #include "drm_internal.h"
> -#include "drm_legacy.h"
>
> /**
> * DOC: master and authentication
> *
> * drm_master is used to track groups of clients with open
> - * primary/legacy device nodes. For every drm_file which has had at
> + * primary device nodes. For every drm_file which has had at
> * least once successfully became the device master (either through the
> * SET_MASTER IOCTL, or implicitly through opening the primary device node
> when
> * no one else is the current master that time) there exists one _master.
> @@ -139,7 +138,6 @@ struct drm_master *drm_master_create(struct drm_device
> *dev)
> return NULL;
>
> kref_init(>refcount);
> - drm_master_legacy_init(master);
> idr_init_base(>magic_map, 1);
> master->dev = dev;
>
> @@ -365,8 +363,6 @@ void drm_master_release(struct drm_file *file_priv)
> if (!drm_is_current_master_locked(file_priv))
> goto out;
>
> - drm_legacy_lock_master_cleanup(dev, master);
> -
> if (dev->master == file_priv->master)
> drm_drop_master(dev, file_priv);
> out:
> @@ -429,8 +425,6 @@ static void drm_master_destroy(struct kref *kref)
> if (drm_core_check_feature(dev, DRIVER_MODESET))
> drm_lease_destroy(master);
>
> - drm_legacy_master_rmmaps(dev, master);
> -
> idr_destroy(>magic_map);
> idr_destroy(>leases);
> idr_destroy(>lessee_idr);
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 535f16e7882e7..979366e68dbb8 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -48,7 +48,6 @@
>
> #include "drm_crtc_internal.h"
> #include "drm_internal.h"
> -#include "drm_legacy.h"
>
> MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl");
> MODULE_DESCRIPTION("DRM shared core routines");
> @@ -585,8 +584,6 @@ static void drm_fs_inode_free(struct inode *inode)
>
> static void drm_dev_init_release(struct drm_device *dev, void *res)
> {
> - drm_legacy_ctxbitmap_cleanup(dev);
> - drm_legacy_remove_map_hash(dev);
> drm_fs_inode_free(dev->anon_inode);
>
> put_device(dev->dev);
> @@ -597,7 +594,6 @@ static void drm_dev_init_release(struct drm_device *dev,
> void *res)
> mutex_destroy(>clientlist_mutex);
> mutex_destroy(>filelist_mutex);
> mutex_destroy(>struct_mutex);
> - drm_legacy_destroy_members(dev);
> }
>
> static int drm_dev_init(struct drm_device *dev,
> @@ -632,7 +628,6 @@ static int drm_dev_init(struct drm_device *dev,
> return -EINVAL;
> }
>
> - drm_legacy_init_members(dev);
> INIT_LIST_HEAD(>filelist);
> INIT_LIST_HEAD(>filelist_internal);
> INIT_LIST_HEAD(>clientlist);
> @@ -673,12 +668,6 @@ static int drm_dev_init(struct drm_device *dev,
> goto err;
> }
>
> - ret = drm_legacy_create_map_hash(dev);
> - if (ret)
> - goto err;
> -
> - drm_legacy_ctxbitmap_init(dev);
> -
> if (drm_core_check_feature(dev, DRIVER_GEM)) {
> ret = drm_gem_init(dev);
> if (ret) {
> @@ -990,9 +979,6 @@ EXPORT_SYMBOL(drm_dev_register);
> */
> void drm_dev_unregister(struct drm_device *dev)
> {
> - if (drm_core_check_feature(dev, DRIVER_LEGACY))
> - drm_lastclose(dev);
> -
> dev->registered = false;
>
> drm_client_dev_unregister(dev);
> @@ -1003,9 +989,6 @@ void drm_dev_unregister(struct drm_device *dev)
> if (dev->driver->unload)
> dev->driver->unload(dev);
>
> - drm_legacy_pci_agp_destroy(dev);
> - drm_legacy_rmmaps(dev);
> -
> remove_compat_control_link(dev);
> drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 5ddaffd325865..987c1a5d1773b 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -47,21 +47,12 @@
>
> #include "drm_crtc_internal.h"
> #include "drm_internal.h"
> -#include "drm_legacy.h"
>
> /* from BKL
Re: [PATCH 09/14] drm: Remove the legacy DRM_IOCTL_MODESET_CTL ioctl
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> DRM drivers with user-space mode setting have been removed in Linux
> v6.3. [1] Now remove the ioctl entry points for these drivers. Invoking
> any of the ioctl ops will unconditionally return -EINVAL to user space.
>
> Invoking DRM_IOCTL_MODESET_CTL is different from the other legacy
> ioctl ops as it returns 0 even without CONFIG_DRM_LEGACY set. From the
> original commit 29935554b384 ("drm: Disallow DRM_IOCTL_MODESET_CTL for
> KMS drivers") it is not apparent how or why the operation differs from
> the others. It is likely just an oversight in commit 61ae227032e7
> ("drm: allow removal of legacy codepaths (v4.1)"), which allowed
> disabling leagacy ioctls in the first place. Still keep this removal
> separate from the other ioctls to allow an easy revert, if necessary.
>
> Signed-off-by: Thomas Zimmermann
> Link: https://patchwork.freedesktop.org/series/111602/ # [1]
Acked-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_internal.h | 2 -
> drivers/gpu/drm/drm_ioctl.c| 2 -
> drivers/gpu/drm/drm_vblank.c | 82 --
> 3 files changed, 86 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index b12c463bc4605..a538a48c7d952 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -117,8 +117,6 @@ void drm_handle_vblank_works(struct drm_vblank_crtc
> *vblank);
> /* IOCTLS */
> int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp);
> -int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data,
> -struct drm_file *file_priv);
>
> /* drm_irq.c */
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index e6c32f76c7452..9c6326b908e74 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -585,8 +585,6 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>
> DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank_ioctl,
> DRM_UNLOCKED),
>
> - DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_legacy_modeset_ctl_ioctl, 0),
> -
> DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop,
> DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
> DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl,
> DRM_RENDER_ALLOW),
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 877e2067534fa..a11f164b2384f 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1574,88 +1574,6 @@ void drm_crtc_vblank_restore(struct drm_crtc *crtc)
> }
> EXPORT_SYMBOL(drm_crtc_vblank_restore);
>
> -static void drm_legacy_vblank_pre_modeset(struct drm_device *dev,
> - unsigned int pipe)
> -{
> - struct drm_vblank_crtc *vblank = >vblank[pipe];
> -
> - /* vblank is not initialized (IRQ not installed ?), or has been freed
> */
> - if (!drm_dev_has_vblank(dev))
> - return;
> -
> - if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
> - return;
> -
> - /*
> -* To avoid all the problems that might happen if interrupts
> -* were enabled/disabled around or between these calls, we just
> -* have the kernel take a reference on the CRTC (just once though
> -* to avoid corrupting the count if multiple, mismatch calls occur),
> -* so that interrupts remain enabled in the interim.
> -*/
> - if (!vblank->inmodeset) {
> - vblank->inmodeset = 0x1;
> - if (drm_vblank_get(dev, pipe) == 0)
> - vblank->inmodeset |= 0x2;
> - }
> -}
> -
> -static void drm_legacy_vblank_post_modeset(struct drm_device *dev,
> - unsigned int pipe)
> -{
> - struct drm_vblank_crtc *vblank = >vblank[pipe];
> -
> - /* vblank is not initialized (IRQ not installed ?), or has been freed
> */
> - if (!drm_dev_has_vblank(dev))
> - return;
> -
> - if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
> - return;
> -
> - if (vblank->inmodeset) {
> - spin_lock_irq(>vbl_lock);
> - drm_reset_vblank_timestamp(dev, pipe);
> - spin_unlock_irq(>vbl_lock);
> -
> - if (vblank->inmodeset & 0x2)
> - drm_vblank_put(dev, pipe);
> -
> - vblank->inmodeset = 0;
> - }
> -}
> -
> -int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data,
> -struct drm_file *file_priv)
> -{
> - struct drm_modeset_ctl *modeset = data;
> - unsigned int pipe;
> -
> - /* If drm_vblank_init() hasn't been called yet, just no-op */
> - if (!drm_dev_has_vblank(dev))
> - return 0;
> -
> - /* KMS drivers handle this internally */
> - if (!drm_core_check_feature(dev,
Re: [PATCH 08/14] drm: Remove entry points for legacy ioctls
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> DRM drivers with user-space mode setting have been removed in Linux
> v6.3. [1] Now remove the ioctl entry points for these drivers. Invoking
> any of the ioctl ops will unconditionally return -EINVAL to user space.
> This has already been the behavior for kernels without CONFIG_DRM_LEGACY
> set.
>
> Signed-off-by: Thomas Zimmermann
> Link: https://patchwork.freedesktop.org/series/111602/ # [1]
Acked-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_ioc32.c | 610
> drivers/gpu/drm/drm_ioctl.c | 57
> 2 files changed, 667 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index c0163cb076ad8..910cadf14756e 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -37,7 +37,6 @@
>
> #include "drm_crtc_internal.h"
> #include "drm_internal.h"
> -#include "drm_legacy.h"
>
> #define DRM_IOCTL_VERSION32DRM_IOWR(0x00, drm_version32_t)
> #define DRM_IOCTL_GET_UNIQUE32 DRM_IOWR(0x01, drm_unique32_t)
> @@ -164,92 +163,6 @@ static int compat_drm_setunique(struct file *file,
> unsigned int cmd,
> return -EINVAL;
> }
>
> -#if IS_ENABLED(CONFIG_DRM_LEGACY)
> -typedef struct drm_map32 {
> - u32 offset; /* Requested physical address (0 for SAREA) */
> - u32 size; /* Requested physical size (bytes) */
> - enum drm_map_type type; /* Type of memory to map */
> - enum drm_map_flags flags; /* Flags */
> - u32 handle; /* User-space: "Handle" to pass to mmap() */
> - int mtrr; /* MTRR slot used */
> -} drm_map32_t;
> -
> -static int compat_drm_getmap(struct file *file, unsigned int cmd,
> -unsigned long arg)
> -{
> - drm_map32_t __user *argp = (void __user *)arg;
> - drm_map32_t m32;
> - struct drm_map map;
> - int err;
> -
> - if (copy_from_user(, argp, sizeof(m32)))
> - return -EFAULT;
> -
> - map.offset = m32.offset;
> - err = drm_ioctl_kernel(file, drm_legacy_getmap_ioctl, , 0);
> - if (err)
> - return err;
> -
> - m32.offset = map.offset;
> - m32.size = map.size;
> - m32.type = map.type;
> - m32.flags = map.flags;
> - m32.handle = ptr_to_compat((void __user *)map.handle);
> - m32.mtrr = map.mtrr;
> - if (copy_to_user(argp, , sizeof(m32)))
> - return -EFAULT;
> - return 0;
> -
> -}
> -
> -static int compat_drm_addmap(struct file *file, unsigned int cmd,
> -unsigned long arg)
> -{
> - drm_map32_t __user *argp = (void __user *)arg;
> - drm_map32_t m32;
> - struct drm_map map;
> - int err;
> -
> - if (copy_from_user(, argp, sizeof(m32)))
> - return -EFAULT;
> -
> - map.offset = m32.offset;
> - map.size = m32.size;
> - map.type = m32.type;
> - map.flags = m32.flags;
> -
> - err = drm_ioctl_kernel(file, drm_legacy_addmap_ioctl, ,
> - DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY);
> - if (err)
> - return err;
> -
> - m32.offset = map.offset;
> - m32.mtrr = map.mtrr;
> - m32.handle = ptr_to_compat((void __user *)map.handle);
> - if (map.handle != compat_ptr(m32.handle))
> - pr_err_ratelimited("compat_drm_addmap truncated handle %p for
> type %d offset %x\n",
> - map.handle, m32.type, m32.offset);
> -
> - if (copy_to_user(argp, , sizeof(m32)))
> - return -EFAULT;
> -
> - return 0;
> -}
> -
> -static int compat_drm_rmmap(struct file *file, unsigned int cmd,
> - unsigned long arg)
> -{
> - drm_map32_t __user *argp = (void __user *)arg;
> - struct drm_map map;
> - u32 handle;
> -
> - if (get_user(handle, >handle))
> - return -EFAULT;
> - map.handle = compat_ptr(handle);
> - return drm_ioctl_kernel(file, drm_legacy_rmmap_ioctl, , DRM_AUTH);
> -}
> -#endif
> -
> typedef struct drm_client32 {
> int idx;/* Which client desired? */
> int auth; /* Is client authenticated? */
> @@ -309,501 +222,6 @@ static int compat_drm_getstats(struct file *file,
> unsigned int cmd,
> return 0;
> }
>
> -#if IS_ENABLED(CONFIG_DRM_LEGACY)
> -typedef struct drm_buf_desc32 {
> - int count; /* Number of buffers of this size */
> - int size;/* Size in bytes */
> - int low_mark;/* Low water mark */
> - int high_mark; /* High water mark */
> - int flags;
> - u32 agp_start; /* Start address in the AGP aperture */
> -} drm_buf_desc32_t;
> -
> -static int compat_drm_addbufs(struct file *file, unsigned int cmd,
> - unsigned long arg)
> -{
>
Re: [PATCH 05/14] accel: Include
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote: > > One of the source files includes via , > which will be removed. Include drm_auth.h directly. > > Signed-off-by: Thomas Zimmermann > Cc: Oded Gabbay Acked-by: Alex Deucher > --- > drivers/accel/drm_accel.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/accel/drm_accel.c b/drivers/accel/drm_accel.c > index 294b572a9c331..24cac4c0274bb 100644 > --- a/drivers/accel/drm_accel.c > +++ b/drivers/accel/drm_accel.c > @@ -11,6 +11,7 @@ > #include > > #include > +#include > #include > #include > #include > -- > 2.42.1 >
Re: [PATCH 03/14] drm: Include
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote: > > One of the source files includes via , > which will be removed. Include drm_auth.h directly. > > Signed-off-by: Thomas Zimmermann Acked-by: Alex Deucher > --- > drivers/gpu/drm/drm_pci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c > index 39d35fc3a43bc..67907dd427214 100644 > --- a/drivers/gpu/drm/drm_pci.c > +++ b/drivers/gpu/drm/drm_pci.c > @@ -29,6 +29,7 @@ > #include > #include > > +#include > #include > #include > #include > -- > 2.42.1 >
Re: [PATCH 01/14] arch/powerpc: Remove legacy DRM drivers from default configs
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> DRM drivers for user-space modesetting have been removed. Do not
> select the respective options in the default configs.
>
> Signed-off-by: Thomas Zimmermann
> Fixes: a276afc19eec ("drm: Remove some obsolete drm pciids(tdfx, mga, i810,
> savage, r128, sis, via)")
> Cc: Cai Huoqing
> Cc: Daniel Vetter
> Cc: Dave Airlie
> Cc: Thomas Zimmermann
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: dri-devel@lists.freedesktop.org
> Cc: # v6.3+
Acked-by: Alex Deucher
> ---
> arch/powerpc/configs/pmac32_defconfig | 2 --
> arch/powerpc/configs/ppc6xx_defconfig | 7 ---
> 2 files changed, 9 deletions(-)
>
> diff --git a/arch/powerpc/configs/pmac32_defconfig
> b/arch/powerpc/configs/pmac32_defconfig
> index 57ded82c28409..e41e7affd2482 100644
> --- a/arch/powerpc/configs/pmac32_defconfig
> +++ b/arch/powerpc/configs/pmac32_defconfig
> @@ -188,8 +188,6 @@ CONFIG_AGP=m
> CONFIG_AGP_UNINORTH=m
> CONFIG_DRM=m
> CONFIG_DRM_RADEON=m
> -CONFIG_DRM_LEGACY=y
> -CONFIG_DRM_R128=m
> CONFIG_FB=y
> CONFIG_FB_OF=y
> CONFIG_FB_CONTROL=y
> diff --git a/arch/powerpc/configs/ppc6xx_defconfig
> b/arch/powerpc/configs/ppc6xx_defconfig
> index f279703425d45..e680cd086f0e8 100644
> --- a/arch/powerpc/configs/ppc6xx_defconfig
> +++ b/arch/powerpc/configs/ppc6xx_defconfig
> @@ -678,13 +678,6 @@ CONFIG_AGP=y
> CONFIG_AGP_UNINORTH=y
> CONFIG_DRM=m
> CONFIG_DRM_RADEON=m
> -CONFIG_DRM_LEGACY=y
> -CONFIG_DRM_TDFX=m
> -CONFIG_DRM_R128=m
> -CONFIG_DRM_MGA=m
> -CONFIG_DRM_SIS=m
> -CONFIG_DRM_VIA=m
> -CONFIG_DRM_SAVAGE=m
> CONFIG_FB=y
> CONFIG_FB_CIRRUS=m
> CONFIG_FB_OF=y
> --
> 2.42.1
>
Re: [PATCH 02/14] drm: Fix TODO list mentioning non-KMS drivers
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote:
>
> Non-KMS drivers have been removed from DRM. Update the TODO list
> accordingly.
>
> Signed-off-by: Thomas Zimmermann
> Fixes: a276afc19eec ("drm: Remove some obsolete drm pciids(tdfx, mga, i810,
> savage, r128, sis, via)")
> Cc: Cai Huoqing
> Cc: Daniel Vetter
> Cc: Dave Airlie
> Cc: Thomas Zimmermann
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Jonathan Corbet
> Cc: dri-devel@lists.freedesktop.org
> Cc: # v6.3+
> Cc: linux-...@vger.kernel.org
Acked-by: Alex Deucher
> ---
> Documentation/gpu/todo.rst | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index b62c7fa0c2bcc..3bdb8787960be 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -337,8 +337,8 @@ connector register/unregister fixes
>
> Level: Intermediate
>
> -Remove load/unload callbacks from all non-DRIVER_LEGACY drivers
>
> +Remove load/unload callbacks
> +
>
> The load/unload callbacks in struct _driver are very much midlayers, plus
> for historical reasons they get the ordering wrong (and we can't fix that)
> @@ -347,8 +347,7 @@ between setting up the _driver structure and calling
> drm_dev_register().
> - Rework drivers to no longer use the load/unload callbacks, directly coding
> the
>load/unload sequence into the driver's probe function.
>
> -- Once all non-DRIVER_LEGACY drivers are converted, disallow the load/unload
> - callbacks for all modern drivers.
> +- Once all drivers are converted, remove the load/unload callbacks.
>
> Contact: Daniel Vetter
>
> --
> 2.42.1
>
Re: [PATCH 07/14] drm/radeon: Do not include
On Wed, Nov 22, 2023 at 7:25 AM Thomas Zimmermann wrote: > > Including is not required by radeon. > > Signed-off-by: Thomas Zimmermann > Cc: Alex Deucher > Cc: "Christian König" > Cc: "Pan, Xinhui" > Cc: amd-...@lists.freedesktop.org Acked-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_drv.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.h > b/drivers/gpu/drm/radeon/radeon_drv.h > index 34a1c73d3938f..02a65971d140d 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.h > +++ b/drivers/gpu/drm/radeon/radeon_drv.h > @@ -33,7 +33,6 @@ > > #include > #include > -#include > > #include "radeon_family.h" > > -- > 2.42.1 >
Re: [PATCH] drm/amd/pm: fix a memleak in aldebaran_tables_init
Applied. Thanks!
On Thu, Nov 23, 2023 at 3:08 AM Dinghao Liu wrote:
>
> When kzalloc() for smu_table->ecc_table fails, we should free
> the previously allocated resources to prevent memleak.
>
> Fixes: edd794208555 ("drm/amd/pm: add message smu to get ecc_table v2")
> Signed-off-by: Dinghao Liu
> ---
> drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 1a6675d70a4b..f1440869d1ce 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -257,8 +257,11 @@ static int aldebaran_tables_init(struct smu_context *smu)
> }
>
> smu_table->ecc_table = kzalloc(tables[SMU_TABLE_ECCINFO].size,
> GFP_KERNEL);
> - if (!smu_table->ecc_table)
> + if (!smu_table->ecc_table) {
> + kfree(smu_table->metrics_table);
> + kfree(smu_table->gpu_metrics_table);
> return -ENOMEM;
> + }
>
> return 0;
> }
> --
> 2.17.1
>
Re: [PATCH v2] Remove custom dumb_map_offset implementation in msm driver
On Tue, Nov 21, 2023 at 5:14 AM Dmitry Baryshkov
wrote:
>
> On Tue, 21 Nov 2023 at 04:26, Rob Clark wrote:
> >
> > On Wed, Nov 15, 2023 at 11:33 AM Dmitry Baryshkov
> > wrote:
> > >
> > > On Wed, 15 Nov 2023 at 20:46, Dipam Turkar wrote:
> > > >
> > > > They are not outdated, my bad. I went through the locks' code and saw
> > > > that they have been updated. But they are probably not necessary here
> > > > as most of the drivers do not use any form of locking in their
> > > > implementations. The generic implementations drm_gem_dumb_map_offset()
> > > > and drm_gem_ttm_dumb_map_offset() do not have any locking mechanisms
> > > > either.
> > >
> > > Excuse me, but this doesn't sound right to me. There are different
> > > drivers with different implementations. So either we'd need a good
> > > explanation of why it is not necessary, or this patch is NAKed.
> >
> > Digging a bit thru history, it looks like commit 0de23977cfeb
> > ("drm/gem: convert to new unified vma manager") made external locking
> > unnecessary, since the vma mgr already had it's own internal locking.
>
> So, should we drop our own locking system?
specifically for _just_ vma_offset_manager/vma_node, we could. But I
think that only amounts to mmap_offset().
BR,
-R
> >
> > BR,
> > -R
> >
> > > >
> > > > Thanks and regards
> > > > Dipam Turkar
> > > >
> > > > On Wed, Nov 15, 2023 at 8:37 PM Dmitry Baryshkov
> > > > wrote:
> > > >>
> > > >> On Wed, 15 Nov 2023 at 16:30, Dipam Turkar
> > > >> wrote:
> > > >> >
> > > >> > Make msm use drm_gem_create_map_offset() instead of its custom
> > > >> > implementation for associating GEM object with a fake offset. Since,
> > > >> > we already have this generic implementation, we don't need the custom
> > > >> > implementation and it is better to standardize the code for GEM based
> > > >> > drivers. This also removes the outdated locking leftovers.
> > > >>
> > > >> Why are they outdated?
> > > >>
> > > >> >
> > > >> > Signed-off-by: Dipam Turkar
> > > >> > ---
> > > >> > drivers/gpu/drm/msm/msm_drv.c | 2 +-
> > > >> > drivers/gpu/drm/msm/msm_gem.c | 21 -
> > > >> > drivers/gpu/drm/msm/msm_gem.h | 2 --
> > > >> > 3 files changed, 1 insertion(+), 24 deletions(-)
> > > >> >
> > > >> > Changes in v2:
> > > >> > Modify commit message to include the absence of internal locking
> > > >> > leftovers
> > > >> > around allocating a fake offset in msm_gem_mmap_offset() in the
> > > >> > generic
> > > >> > implementation drm_gem_create_map_offset().
> > > >> >
> > > >> > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> > > >> > b/drivers/gpu/drm/msm/msm_drv.c
> > > >> > index a428951ee539..86a15992c717 100644
> > > >> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > >> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > >> > @@ -1085,7 +1085,7 @@ static const struct drm_driver msm_driver = {
> > > >> > .open = msm_open,
> > > >> > .postclose = msm_postclose,
> > > >> > .dumb_create= msm_gem_dumb_create,
> > > >> > - .dumb_map_offset= msm_gem_dumb_map_offset,
> > > >> > + .dumb_map_offset= drm_gem_dumb_map_offset,
> > > >> > .gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
> > > >> > #ifdef CONFIG_DEBUG_FS
> > > >> > .debugfs_init = msm_debugfs_init,
> > > >> > diff --git a/drivers/gpu/drm/msm/msm_gem.c
> > > >> > b/drivers/gpu/drm/msm/msm_gem.c
> > > >> > index db1e748daa75..489694ef79cb 100644
> > > >> > --- a/drivers/gpu/drm/msm/msm_gem.c
> > > >> > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > > >> > @@ -671,27 +671,6 @@ int msm_gem_dumb_create(struct drm_file *file,
> > > >> > struct drm_device *dev,
> > > >> > MSM_BO_SCANOUT | MSM_BO_WC, >handle,
> > > >> > "dumb");
> > > >> > }
> > > >> >
> > > >> > -int msm_gem_dumb_map_offset(struct drm_file *file, struct
> > > >> > drm_device *dev,
> > > >> > - uint32_t handle, uint64_t *offset)
> > > >> > -{
> > > >> > - struct drm_gem_object *obj;
> > > >> > - int ret = 0;
> > > >> > -
> > > >> > - /* GEM does all our handle to object mapping */
> > > >> > - obj = drm_gem_object_lookup(file, handle);
> > > >> > - if (obj == NULL) {
> > > >> > - ret = -ENOENT;
> > > >> > - goto fail;
> > > >> > - }
> > > >> > -
> > > >> > - *offset = msm_gem_mmap_offset(obj);
> > > >> > -
> > > >> > - drm_gem_object_put(obj);
> > > >> > -
> > > >> > -fail:
> > > >> > - return ret;
> > > >> > -}
> > > >> > -
> > > >> > static void *get_vaddr(struct drm_gem_object *obj, unsigned madv)
> > > >> > {
> > > >> > struct msm_gem_object *msm_obj = to_msm_bo(obj);
> > > >> > diff --git a/drivers/gpu/drm/msm/msm_gem.h
> > > >> > b/drivers/gpu/drm/msm/msm_gem.h
> > > >> > index 8ddef5443140..dc74a0ef865d 100644
> > > >> > --- a/drivers/gpu/drm/msm/msm_gem.h
> > > >> > +++ b/drivers/gpu/drm/msm/msm_gem.h
> > > >> > @@ -139,8 +139,6 @@
Re: [Nouveau] [PATCH 3/3] nouveau/gsp: add some basic registry entries.
On Tue, Oct 31, 2023 at 12:20 AM Dave Airlie wrote:
> rpc->size = sizeof(*rpc);
> - rpc->numEntries = 1;
> - rpc->entries[0].nameOffset = offsetof(typeof(*rpc), entries[1]);
> - rpc->entries[0].type = 1;
> - rpc->entries[0].data = 0;
> - rpc->entries[0].length = 4;
> -
> - strings = (char *)>entries[1];
> - strings[0] = '\0';
> + rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
> +
> + str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
> + strings = (char *)>entries[NV_GSP_REG_NUM_ENTRIES];
> + for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
> + int name_len = strlen(r535_registry_entries[i].name) + 1;
> + rpc->entries[i].nameOffset = str_offset;
> + rpc->entries[i].type = 1;
> + rpc->entries[i].data = r535_registry_entries[i].value;
> + rpc->entries[i].length = 4;
> + memcpy(strings, r535_registry_entries[i].name, name_len);
> + strings += name_len;
> + str_offset += name_len;
> + }
I'm working on a patch that replaces this code with a
dynamically-generated registry so that we can set registry keys via
the driver's command-line (like the Nvidia driver). However, you have
a bug here. rpc->size must be the total size of the RPC, including
all the PACKED_REGISTRY_ENTRY structs and the strings that follow
them. You can see this by looking at RmPackageRegistry() and
regCountEntriesAndSize() in OpenRM.
Re: [Intel-gfx] [PATCH v1 1/1] drm/i915/gt: Dont wait forever when idling in suspend
On Tue, Nov 14, 2023 at 08:22:27AM -0800, Alan Previn wrote:
> When suspending, add a timeout when calling
> intel_gt_pm_wait_for_idle else if we have a leaked
> wakeref (which would be indicative of a bug elsewhere
> in the driver), driver will at exit the suspend-resume
> cycle, after the kernel detects the held reference and
> prints a message to abort suspending instead of hanging
> in the kernel forever which then requires serial connection
> or ramoops dump to debug further.
>
> Signed-off-by: Alan Previn
> Reviewed-by: Rodrigo Vivi
> Tested-by: Mousumi Jana
could you please rebase this on top of recent drm-tip and resend?
I got a conflict while trying to apply on drm-intel-gt-next.
As I had stated in another thread, I believe we should go further
and block the suspend and have a clean return to normal operation.
But anyway, I agree this is already a good and necessary improvement
because being in the dark if some bad case like this happens is
the worst thing. So this patch is already an improvement anyway.
again:
Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_pm.c | 7 ++-
> drivers/gpu/drm/i915/gt/intel_gt_pm.h | 7 ++-
> drivers/gpu/drm/i915/intel_wakeref.c | 14 ++
> drivers/gpu/drm/i915/intel_wakeref.h | 6 --
> 5 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 40687806d22a..ffef963037f2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -686,7 +686,7 @@ void intel_engines_release(struct intel_gt *gt)
> if (!engine->release)
> continue;
>
> - intel_wakeref_wait_for_idle(>wakeref);
> + intel_wakeref_wait_for_idle(>wakeref, 0);
> GEM_BUG_ON(intel_engine_pm_is_awake(engine));
>
> engine->release(engine);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index f5899d503e23..25cb39ba9fdf 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -306,6 +306,8 @@ int intel_gt_resume(struct intel_gt *gt)
>
> static void wait_for_suspend(struct intel_gt *gt)
> {
> + int final_timeout_ms = (I915_GT_SUSPEND_IDLE_TIMEOUT * 10);
> +
> if (!intel_gt_pm_is_awake(gt))
> return;
>
> @@ -318,7 +320,10 @@ static void wait_for_suspend(struct intel_gt *gt)
> intel_gt_retire_requests(gt);
> }
>
> - intel_gt_pm_wait_for_idle(gt);
> + /* we are suspending, so we shouldn't be waiting forever */
> + if (intel_gt_pm_wait_timeout_for_idle(gt, final_timeout_ms) ==
> -ETIMEDOUT)
> + gt_warn(gt, "bailing from %s after %d milisec timeout\n",
> + __func__, final_timeout_ms);
> }
>
> void intel_gt_suspend_prepare(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
> b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
> index b1eeb5b33918..1757ca4c3077 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
> @@ -68,7 +68,12 @@ static inline void intel_gt_pm_might_put(struct intel_gt
> *gt)
>
> static inline int intel_gt_pm_wait_for_idle(struct intel_gt *gt)
> {
> - return intel_wakeref_wait_for_idle(>wakeref);
> + return intel_wakeref_wait_for_idle(>wakeref, 0);
> +}
> +
> +static inline int intel_gt_pm_wait_timeout_for_idle(struct intel_gt *gt, int
> timeout_ms)
> +{
> + return intel_wakeref_wait_for_idle(>wakeref, timeout_ms);
> }
>
> void intel_gt_pm_init_early(struct intel_gt *gt);
> diff --git a/drivers/gpu/drm/i915/intel_wakeref.c
> b/drivers/gpu/drm/i915/intel_wakeref.c
> index 623a69089386..f2611c65246b 100644
> --- a/drivers/gpu/drm/i915/intel_wakeref.c
> +++ b/drivers/gpu/drm/i915/intel_wakeref.c
> @@ -113,14 +113,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
>"wakeref.work", >work, 0);
> }
>
> -int intel_wakeref_wait_for_idle(struct intel_wakeref *wf)
> +int intel_wakeref_wait_for_idle(struct intel_wakeref *wf, int timeout_ms)
> {
> - int err;
> + int err = 0;
>
> might_sleep();
>
> - err = wait_var_event_killable(>wakeref,
> - !intel_wakeref_is_active(wf));
> + if (!timeout_ms)
> + err = wait_var_event_killable(>wakeref,
> + !intel_wakeref_is_active(wf));
> + else if (wait_var_event_timeout(>wakeref,
> + !intel_wakeref_is_active(wf),
> + msecs_to_jiffies(timeout_ms)) < 1)
> + err = -ETIMEDOUT;
> +
> if (err)
> return err;
>
> diff --git a/drivers/gpu/drm/i915/intel_wakeref.h
>
Re: [PATCH v2 1/1] drm/i915/pxp: Add missing tag for Wa_14019159160
On Wed, Nov 22, 2023 at 12:30:03PM -0800, Alan Previn wrote:
> Add missing tag for "Wa_14019159160 - Case 2" (for existing
> PXP code that ensures run alone mode bit is set to allow
> PxP-decryption.
>
> v2: - Fix WA id number (John Harrison).
> - Improve comments and code to be specific
>for the targetted platforms (John Harrison)
>
> Signed-off-by: Alan Previn
> ---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 7c367ba8d9dc..2959dfed2aa0 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -863,11 +863,13 @@ static bool ctx_needs_runalone(const struct
> intel_context *ce)
> bool ctx_is_protected = false;
>
> /*
> - * On MTL and newer platforms, protected contexts require setting
> - * the LRC run-alone bit or else the encryption will not happen.
> + * Wa_14019159160 - Case 2: mtl
> + * On some platforms, protected contexts require setting
> + * the LRC run-alone bit or else the encryption/decryption will not
> happen.
> + * NOTE: Case 2 only applies to PXP use-case of said workaround.
>*/
hmm, interesting enough, on the wa description I read that it is incomplete for
MTL/ARL
and something about a fuse bit. We should probably chase for some clarification
in the
HSD?!
> - if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
> - (ce->engine->class == COMPUTE_CLASS || ce->engine->class ==
> RENDER_CLASS)) {
> + if (IS_METEORLAKE(ce->engine->i915) && (ce->engine->class ==
> COMPUTE_CLASS ||
> + ce->engine->class ==
> RENDER_CLASS)) {
This check now excludes the ARL with the same IP, no?!
> rcu_read_lock();
> gem_ctx = rcu_dereference(ce->gem_context);
> if (gem_ctx)
>
> base-commit: 5429d55de723544dfc0630cf39d96392052b27a1
> --
> 2.39.0
>
[PATCH v3 1/1] drm/i915/pxp: Add missing tag for Wa_14019159160
Add missing tag for "Wa_14019159160 - Case 2" (for existing
PXP code that ensures run alone mode bit is set to allow
PxP-decryption.
v3: - Check targeted platforms using IP_VAL. (John Harrison)
v2: - Fix WA id number (John Harrison).
- Improve comments and code to be specific
for the targeted platforms (John Harrison)
Signed-off-by: Alan Previn
---
drivers/gpu/drm/i915/gt/intel_lrc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 7c367ba8d9dc..1152cf25d578 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -863,10 +863,12 @@ static bool ctx_needs_runalone(const struct intel_context
*ce)
bool ctx_is_protected = false;
/*
-* On MTL and newer platforms, protected contexts require setting
-* the LRC run-alone bit or else the encryption will not happen.
+* Wa_14019159160 - Case 2: mtl
+* On some platforms, protected contexts require setting
+* the LRC run-alone bit or else the encryption/decryption will not
happen.
+* NOTE: Case 2 only applies to PXP use-case of said workaround.
*/
- if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
+ if (GRAPHICS_VER_FULL(ce->engine->i915) == IP_VER(12, 70) &&
(ce->engine->class == COMPUTE_CLASS || ce->engine->class ==
RENDER_CLASS)) {
rcu_read_lock();
gem_ctx = rcu_dereference(ce->gem_context);
base-commit: 5429d55de723544dfc0630cf39d96392052b27a1
--
2.39.0
Re: [Nouveau] [PATCH] nouveau/gsp/r535: remove a stray unlock in r535_gsp_rpc_send()
On Mon, 2023-11-27 at 15:56 +0300, Dan Carpenter wrote:
> This unlock doesn't belong here and it leads to a double unlock in
> the caller, r535_gsp_rpc_push().
>
> Fixes: 176fdcbddfd2 ("drm/nouveau/gsp/r535: add support for booting GSP-
> RM")
> Signed-off-by: Dan Carpenter
Nice catch!
Reviewed-by: Timur Tabi
Re: [PATCH v5] Documentation/gpu: VM_BIND locking document
On 11/22/23 08:49, Thomas Hellström wrote: On 11/21/23 14:56, Boris Brezillon wrote: On Tue, 21 Nov 2023 11:40:46 +0100 Thomas Hellström wrote: Add the first version of the VM_BIND locking document which is intended to be part of the xe driver upstreaming agreement. The document describes and discuss the locking used during exec- functions, evicton and for userptr gpu-vmas. Intention is to be using the same nomenclature as the drm-vm-bind-async.rst. v2: - s/gvm/gpu_vm/g (Rodrigo Vivi) - Clarify the userptr seqlock with a pointer to mm/mmu_notifier.c (Rodrigo Vivi) - Adjust commit message accordingly. - Add SPDX license header. v3: - Large update to align with the drm_gpuvm manager locking - Add "Efficient userptr gpu_vma exec function iteration" section - Add "Locking at bind- and unbind time" section. v4: - Fix tabs vs space errors by untabifying (Rodrigo Vivi) - Minor style fixes and typos (Rodrigo Vivi) - Clarify situations where stale GPU mappings are occurring and how access through these mappings are blocked. (Rodrigo Vivi) - Insert into the toctree in implementation_guidelines.rst v5: - Add a section about recoverable page-faults. - Use local references to other documentation where possible (Bagas Sanjaya) - General documentation fixes and typos (Danilo Krummrich and Boris Brezillon) - Improve the documentation around locks that need to be grabbed from the dm-fence critical section (Boris Brezillon) - Add more references to the DRM GPUVM helpers (Danilo Krummrich and Boriz Brezillon) - Update the rfc/xe.rst document. Cc: Rodrigo Vivi Signed-off-by: Thomas Hellström Still have a few comments (see below), and in general, I find some sentences very long, which has the tendency of confusing me (always trying to figure out what was the main point, what the pronouns are referring to, etc). Anyway, I think it's better to have something imperfect than nothing at all, so here is my Reviewed-by: Boris Brezillon Feel free to add it even if you decide to ignore my comments. Thanks for reviewing, Boris! I'll make a final version incorporating much of the comments and suggestions, much appreciated. I still think, though, that in principle the referral between gpuvm and this document should be the other way around, or it should all sit in gpuvm. In any case this is an initial version and as more comments and suggestions land, we can certainly update. I think if we agree that GPUVM should be the common component that we recommend drivers to use, we should reference to GPUVM whenever possible. However, I'm not sure whether we'd want to dedicate *all* documentation to GPUVM. Since the topic is rather complex, I can see that it might be beneficial to do both, discuss it from a more abstract point of view and document the corresponding common component. Reviewed-by: Danilo Krummrich Thanks, Thomas
Re: [Intel-xe] [PATCH v5] Documentation/gpu: VM_BIND locking document
On Tue, Nov 21, 2023 at 11:40:46AM +0100, Thomas Hellström wrote: > Add the first version of the VM_BIND locking document which is > intended to be part of the xe driver upstreaming agreement. > > The document describes and discuss the locking used during exec- > functions, evicton and for userptr gpu-vmas. Intention is to be using the > same nomenclature as the drm-vm-bind-async.rst. > > v2: > - s/gvm/gpu_vm/g (Rodrigo Vivi) > - Clarify the userptr seqlock with a pointer to mm/mmu_notifier.c > (Rodrigo Vivi) > - Adjust commit message accordingly. > - Add SPDX license header. > > v3: > - Large update to align with the drm_gpuvm manager locking > - Add "Efficient userptr gpu_vma exec function iteration" section > - Add "Locking at bind- and unbind time" section. > > v4: > - Fix tabs vs space errors by untabifying (Rodrigo Vivi) > - Minor style fixes and typos (Rodrigo Vivi) > - Clarify situations where stale GPU mappings are occurring and how > access through these mappings are blocked. (Rodrigo Vivi) > - Insert into the toctree in implementation_guidelines.rst > > v5: > - Add a section about recoverable page-faults. > - Use local references to other documentation where possible > (Bagas Sanjaya) > - General documentation fixes and typos (Danilo Krummrich and > Boris Brezillon) > - Improve the documentation around locks that need to be grabbed from the > dm-fence critical section (Boris Brezillon) > - Add more references to the DRM GPUVM helpers (Danilo Krummrich and > Boriz Brezillon) > - Update the rfc/xe.rst document. > > Cc: Rodrigo Vivi > Signed-off-by: Thomas Hellström First of all, with Bagas and Boris latest suggestions, already few free to use: Reviewed-by: Rodrigo Vivi But a few minor comments below. Mostly trying to address Boris feeling of long sentences. However, take them with a grain of salt since I'm not a native english speaker. :) > --- > Documentation/core-api/pin_user_pages.rst | 2 + > Documentation/gpu/drm-mm.rst | 4 + > Documentation/gpu/drm-vm-bind-locking.rst | 579 ++ > .../gpu/implementation_guidelines.rst | 1 + > Documentation/gpu/rfc/xe.rst | 5 + > 5 files changed, 591 insertions(+) > create mode 100644 Documentation/gpu/drm-vm-bind-locking.rst > > diff --git a/Documentation/core-api/pin_user_pages.rst > b/Documentation/core-api/pin_user_pages.rst > index d3c1f6d8c0e0..6b5f7e6e7155 100644 > --- a/Documentation/core-api/pin_user_pages.rst > +++ b/Documentation/core-api/pin_user_pages.rst > @@ -153,6 +153,8 @@ NOTE: Some pages, such as DAX pages, cannot be pinned > with longterm pins. That's > because DAX pages do not have a separate page cache, and so "pinning" implies > locking down file system blocks, which is not (yet) supported in that way. > > +.. _mmu-notifier-registration-case: > + > CASE 3: MMU notifier registration, with or without page faulting hardware > - > Device drivers can pin pages via get_user_pages*(), and register for mmu > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index acc5901ac840..d55751cad67c 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -466,6 +466,8 @@ DRM MM Range Allocator Function References > .. kernel-doc:: drivers/gpu/drm/drm_mm.c > :export: > > +.. _drm_gpuvm: > + > DRM GPUVM > = > > @@ -481,6 +483,8 @@ Split and Merge > .. kernel-doc:: drivers/gpu/drm/drm_gpuvm.c > :doc: Split and Merge > > +.. _drm_gpuvm_locking: > + > Locking > --- > > diff --git a/Documentation/gpu/drm-vm-bind-locking.rst > b/Documentation/gpu/drm-vm-bind-locking.rst > new file mode 100644 > index ..a7d08eccd50e > --- /dev/null > +++ b/Documentation/gpu/drm-vm-bind-locking.rst > @@ -0,0 +1,579 @@ > +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT) > + > +=== > +VM_BIND locking > +=== > + > +This document attempts to describe what's needed to get VM_BIND locking > right, > +including the userptr mmu_notifier locking and it also discusses some including the userptr mmu_notifier locking. It also discusses some > +optimizations to get rid of the looping through of all userptr mappings and > +external / shared object mappings that is needed in the simplest > +implementation. In addition there is a section describing the VM_BIND locking In addition, > +required for implementing recoverable pagefaults. > + > +The DRM GPUVM set of helpers > + > + > +There is a set of helpers for drivers implementing VM_BIND, and this > +set of helpers implements much, but not all of the locking described > +in this document. In particular it is currently lacking a userptr In particular, > +implementation. Please refer to the :ref:`DRM GPUVM documentation > +`. This document does not intend to describe the DRM GPUVM > +implementation in detail,
[PATCH v3 16/17] drm/v3d: Create a CPU job extension for the reset performance query job
A CPU job is a type of job that performs operations that requires CPU
intervention. A reset performance query job is a job that resets the
performance queries by resetting the values of the perfmons. Moreover,
we also reset the syncobjs related to the availability of the query.
So, create a user extension for the CPU job that enables the creation
of a reset performance job. This user extension will allow the creation of
a CPU job that resets the perfmons values and resets the availability syncobj.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.h| 28
drivers/gpu/drm/v3d/v3d_sched.c | 37
drivers/gpu/drm/v3d/v3d_submit.c | 73
include/uapi/drm/v3d_drm.h | 30 +
4 files changed, 168 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 5058a354fffd..0f7f80ad8d88 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -321,6 +321,7 @@ enum v3d_cpu_job_type {
V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY,
V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY,
V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY,
+ V3D_CPU_JOB_TYPE_RESET_PERFORMANCE_QUERY,
};
struct v3d_timestamp_query {
@@ -331,6 +332,18 @@ struct v3d_timestamp_query {
struct drm_syncobj *syncobj;
};
+/* Number of perfmons required to handle all supported performance counters */
+#define V3D_MAX_PERFMONS DIV_ROUND_UP(V3D_PERFCNT_NUM, \
+ DRM_V3D_MAX_PERF_COUNTERS)
+
+struct v3d_performance_query {
+ /* Performance monitor IDs for this query */
+ u32 kperfmon_ids[V3D_MAX_PERFMONS];
+
+ /* Syncobj that indicates the query availability */
+ struct drm_syncobj *syncobj;
+};
+
struct v3d_indirect_csd_info {
/* Indirect CSD */
struct v3d_csd_job *job;
@@ -362,6 +375,19 @@ struct v3d_timestamp_query_info {
u32 count;
};
+struct v3d_performance_query_info {
+ struct v3d_performance_query *queries;
+
+ /* Number of performance queries */
+ u32 count;
+
+ /* Number of performance monitors related to that query pool */
+ u32 nperfmons;
+
+ /* Number of performance counters related to that query pool */
+ u32 ncounters;
+};
+
struct v3d_copy_query_results_info {
/* Define if should write to buffer using 64 or 32 bits */
bool do_64bit;
@@ -389,6 +415,8 @@ struct v3d_cpu_job {
struct v3d_timestamp_query_info timestamp_query;
struct v3d_copy_query_results_info copy;
+
+ struct v3d_performance_query_info performance_query;
};
typedef void (*v3d_cpu_job_fn)(struct v3d_cpu_job *);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 07c897cd3423..452c4a1db52e 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -78,6 +78,7 @@ v3d_cpu_job_free(struct drm_sched_job *sched_job)
{
struct v3d_cpu_job *job = to_cpu_job(sched_job);
struct v3d_timestamp_query_info *timestamp_query =
>timestamp_query;
+ struct v3d_performance_query_info *performance_query =
>performance_query;
if (timestamp_query->queries) {
for (int i = 0; i < timestamp_query->count; i++)
@@ -85,6 +86,12 @@ v3d_cpu_job_free(struct drm_sched_job *sched_job)
kvfree(timestamp_query->queries);
}
+ if (performance_query->queries) {
+ for (int i = 0; i < performance_query->count; i++)
+ drm_syncobj_put(performance_query->queries[i].syncobj);
+ kvfree(performance_query->queries);
+ }
+
v3d_job_cleanup(>base);
}
@@ -361,6 +368,7 @@ v3d_reset_timestamp_queries(struct v3d_cpu_job *job)
v3d_put_bo_vaddr(bo);
}
+
static void
write_to_buffer(void *dst, u32 idx, bool do_64bit, u64 value)
{
@@ -414,11 +422,40 @@ v3d_copy_query_results(struct v3d_cpu_job *job)
v3d_put_bo_vaddr(bo);
}
+static void
+v3d_reset_performance_queries(struct v3d_cpu_job *job)
+{
+ struct v3d_performance_query_info *performance_query =
>performance_query;
+ struct v3d_file_priv *v3d_priv = job->base.file->driver_priv;
+ struct v3d_dev *v3d = job->base.v3d;
+ struct v3d_perfmon *perfmon;
+
+ for (int i = 0; i < performance_query->count; i++) {
+ for (int j = 0; j < performance_query->nperfmons; j++) {
+ perfmon = v3d_perfmon_find(v3d_priv,
+
performance_query->queries[i].kperfmon_ids[j]);
+ if (!perfmon) {
+ DRM_DEBUG("Failed to find perfmon.");
+ continue;
+ }
+
+ v3d_perfmon_stop(v3d, perfmon, false);
+
+ memset(perfmon->values, 0, perfmon->ncounters *
sizeof(u64));
+
+
[PATCH v3 15/17] drm/v3d: Create a CPU job extension to copy timestamp query to a buffer
A CPU job is a type of job that performs operations that requires CPU
intervention. A copy timestamp query job is a job that copy the complete
or partial result of a query to a buffer. As V3D doesn't provide any
mechanism to obtain a timestamp from the GPU, it is a job that needs
CPU intervention.
So, create a user extension for the CPU job that enables the creation
of a copy timestamp query job. This user extension will allow the creation
of a CPU job that copy the results of a timestamp query to a BO with the
possibility to indicate the timestamp availability with a availability bit.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.h| 20 +
drivers/gpu/drm/v3d/v3d_sched.c | 53
drivers/gpu/drm/v3d/v3d_submit.c | 69
include/uapi/drm/v3d_drm.h | 45 +
4 files changed, 187 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 3988407635ed..5058a354fffd 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -320,6 +320,7 @@ enum v3d_cpu_job_type {
V3D_CPU_JOB_TYPE_INDIRECT_CSD = 1,
V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY,
V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY,
+ V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY,
};
struct v3d_timestamp_query {
@@ -361,6 +362,23 @@ struct v3d_timestamp_query_info {
u32 count;
};
+struct v3d_copy_query_results_info {
+ /* Define if should write to buffer using 64 or 32 bits */
+ bool do_64bit;
+
+ /* Define if it can write to buffer even if the query is not available
*/
+ bool do_partial;
+
+ /* Define if it should write availability bit to buffer */
+ bool availability_bit;
+
+ /* Offset of the copy buffer in the BO */
+ u32 offset;
+
+ /* Stride of the copy buffer in the BO */
+ u32 stride;
+};
+
struct v3d_cpu_job {
struct v3d_job base;
@@ -369,6 +387,8 @@ struct v3d_cpu_job {
struct v3d_indirect_csd_info indirect_csd;
struct v3d_timestamp_query_info timestamp_query;
+
+ struct v3d_copy_query_results_info copy;
};
typedef void (*v3d_cpu_job_fn)(struct v3d_cpu_job *);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 3a435f621b9b..07c897cd3423 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -361,11 +361,64 @@ v3d_reset_timestamp_queries(struct v3d_cpu_job *job)
v3d_put_bo_vaddr(bo);
}
+static void
+write_to_buffer(void *dst, u32 idx, bool do_64bit, u64 value)
+{
+ if (do_64bit) {
+ u64 *dst64 = (u64 *) dst;
+ dst64[idx] = value;
+ } else {
+ u32 *dst32 = (u32 *) dst;
+ dst32[idx] = (u32) value;
+ }
+}
+
+static void
+v3d_copy_query_results(struct v3d_cpu_job *job)
+{
+ struct v3d_timestamp_query_info *timestamp_query =
>timestamp_query;
+ struct v3d_timestamp_query *queries = timestamp_query->queries;
+ struct v3d_bo *bo = to_v3d_bo(job->base.bo[0]);
+ struct v3d_bo *timestamp = to_v3d_bo(job->base.bo[1]);
+ struct v3d_copy_query_results_info *copy = >copy;
+ struct dma_fence *fence;
+ u8 *query_addr;
+ bool available, write_result;
+ u8 *data;
+ int i;
+
+ v3d_get_bo_vaddr(bo);
+ v3d_get_bo_vaddr(timestamp);
+
+ data = ((u8 *) bo->vaddr) + copy->offset;
+
+ for (i = 0; i < timestamp_query->count; i++) {
+ fence = drm_syncobj_fence_get(queries[i].syncobj);
+ available = fence ? dma_fence_is_signaled(fence) : false;
+
+ write_result = available || copy->do_partial;
+ if (write_result) {
+ query_addr = ((u8 *) timestamp->vaddr) +
queries[i].offset;
+ write_to_buffer(data, 0, copy->do_64bit, *((u64 *)
query_addr));
+ }
+
+ if (copy->availability_bit)
+ write_to_buffer(data, 1, copy->do_64bit, available ? 1u
: 0u);
+
+ data += copy->stride;
+
+ dma_fence_put(fence);
+ }
+
+ v3d_put_bo_vaddr(timestamp);
+ v3d_put_bo_vaddr(bo);
+}
static const v3d_cpu_job_fn cpu_job_function[] = {
[V3D_CPU_JOB_TYPE_INDIRECT_CSD] =
v3d_rewrite_csd_job_wg_counts_from_indirect,
[V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY] = v3d_timestamp_query,
[V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY] = v3d_reset_timestamp_queries,
+ [V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY] = v3d_copy_query_results,
};
static struct dma_fence *
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 1c719416e26a..bafd49c6440c 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -539,6 +539,71 @@ v3d_get_cpu_reset_timestamp_params(struct drm_file
*file_priv,
return 0;
}
+/* Get data for the copy
[PATCH v3 14/17] drm/v3d: Create a CPU job extension for the reset timestamp job
A CPU job is a type of job that performs operations that requires CPU
intervention. A reset timestamp job is a job that resets the timestamp
queries based on the value offset of the first query. As V3D doesn't
provide any mechanism to obtain a timestamp from the GPU, it is a job
that needs CPU intervention.
So, create a user extension for the CPU job that enables the creation
of a reset timestamp job. This user extension will allow the creation of
a CPU job that resets the timestamp value in the timestamp BO and resets
the availability syncobj.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.h| 1 +
drivers/gpu/drm/v3d/v3d_sched.c | 21 +
drivers/gpu/drm/v3d/v3d_submit.c | 52
include/uapi/drm/v3d_drm.h | 27 +
4 files changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index dd86e745c260..3988407635ed 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -319,6 +319,7 @@ struct v3d_csd_job {
enum v3d_cpu_job_type {
V3D_CPU_JOB_TYPE_INDIRECT_CSD = 1,
V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY,
+ V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY,
};
struct v3d_timestamp_query {
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index f4cffefb6398..3a435f621b9b 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -342,9 +342,30 @@ v3d_timestamp_query(struct v3d_cpu_job *job)
v3d_put_bo_vaddr(bo);
}
+static void
+v3d_reset_timestamp_queries(struct v3d_cpu_job *job)
+{
+ struct v3d_timestamp_query_info *timestamp_query =
>timestamp_query;
+ struct v3d_timestamp_query *queries = timestamp_query->queries;
+ struct v3d_bo *bo = to_v3d_bo(job->base.bo[0]);
+ u8 *value_addr;
+
+ v3d_get_bo_vaddr(bo);
+
+ for (int i = 0; i < timestamp_query->count; i++) {
+ value_addr = ((u8 *) bo->vaddr) + queries[i].offset;
+ *((u64 *) value_addr) = 0;
+
+ drm_syncobj_replace_fence(queries[i].syncobj, NULL);
+ }
+
+ v3d_put_bo_vaddr(bo);
+}
+
static const v3d_cpu_job_fn cpu_job_function[] = {
[V3D_CPU_JOB_TYPE_INDIRECT_CSD] =
v3d_rewrite_csd_job_wg_counts_from_indirect,
[V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY] = v3d_timestamp_query,
+ [V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY] = v3d_reset_timestamp_queries,
};
static struct dma_fence *
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 83e029e786ea..1c719416e26a 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -491,6 +491,54 @@ v3d_get_cpu_timestamp_query_params(struct drm_file
*file_priv,
return 0;
}
+static int
+v3d_get_cpu_reset_timestamp_params(struct drm_file *file_priv,
+ struct drm_v3d_extension __user *ext,
+ struct v3d_cpu_job *job)
+{
+ u32 __user *syncs;
+ struct drm_v3d_reset_timestamp_query reset;
+
+ if (!job) {
+ DRM_DEBUG("CPU job extension was attached to a GPU job.\n");
+ return -EINVAL;
+ }
+
+ if (job->job_type) {
+ DRM_DEBUG("Two CPU job extensions were added to the same CPU
job.\n");
+ return -EINVAL;
+ }
+
+ if (copy_from_user(, ext, sizeof(reset)))
+ return -EFAULT;
+
+ job->job_type = V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY;
+
+ job->timestamp_query.queries = kvmalloc_array(reset.count,
+ sizeof(struct
v3d_timestamp_query),
+ GFP_KERNEL);
+ if (!job->timestamp_query.queries)
+ return -ENOMEM;
+
+ syncs = u64_to_user_ptr(reset.syncs);
+
+ for (int i = 0; i < reset.count; i++) {
+ u32 sync;
+
+ job->timestamp_query.queries[i].offset = reset.offset + 8 * i;
+
+ if (copy_from_user(, syncs++, sizeof(sync))) {
+ kvfree(job->timestamp_query.queries);
+ return -EFAULT;
+ }
+
+ job->timestamp_query.queries[i].syncobj =
drm_syncobj_find(file_priv, sync);
+ }
+ job->timestamp_query.count = reset.count;
+
+ return 0;
+}
+
/* Whenever userspace sets ioctl extensions, v3d_get_extensions parses data
* according to the extension id (name).
*/
@@ -522,6 +570,9 @@ v3d_get_extensions(struct drm_file *file_priv,
case DRM_V3D_EXT_ID_CPU_TIMESTAMP_QUERY:
ret = v3d_get_cpu_timestamp_query_params(file_priv,
user_ext, job);
break;
+ case DRM_V3D_EXT_ID_CPU_RESET_TIMESTAMP_QUERY:
+ ret = v3d_get_cpu_reset_timestamp_params(file_priv,
user_ext, job);
+ break;
[PATCH v3 17/17] drm/v3d: Create a CPU job extension for the copy performance query job
A CPU job is a type of job that performs operations that requires CPU
intervention. A copy performance query job is a job that copy the complete
or partial result of a query to a buffer. In order to copy the result of
a performance query to a buffer, we need to get the values from the
performance monitors.
So, create a user extension for the CPU job that enables the creation
of a copy performance query job. This user extension will allow the creation
of a CPU job that copy the results of a performance query to a BO with the
possibility to indicate the availability with a availability bit.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.h| 1 +
drivers/gpu/drm/v3d/v3d_sched.c | 66 +
drivers/gpu/drm/v3d/v3d_submit.c | 82
include/uapi/drm/v3d_drm.h | 50 +++
4 files changed, 199 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 0f7f80ad8d88..3c7d58866570 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -322,6 +322,7 @@ enum v3d_cpu_job_type {
V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY,
V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY,
V3D_CPU_JOB_TYPE_RESET_PERFORMANCE_QUERY,
+ V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY,
};
struct v3d_timestamp_query {
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 452c4a1db52e..203c32ed99d4 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -450,12 +450,78 @@ v3d_reset_performance_queries(struct v3d_cpu_job *job)
}
}
+static void
+v3d_write_performance_query_result(struct v3d_cpu_job *job, void *data, u32
query)
+{
+ struct v3d_performance_query_info *performance_query =
>performance_query;
+ struct v3d_copy_query_results_info *copy = >copy;
+ struct v3d_file_priv *v3d_priv = job->base.file->driver_priv;
+ struct v3d_dev *v3d = job->base.v3d;
+ struct v3d_perfmon *perfmon;
+ u64 counter_values[V3D_PERFCNT_NUM];
+
+ for (int i = 0; i < performance_query->nperfmons; i++) {
+ perfmon = v3d_perfmon_find(v3d_priv,
+
performance_query->queries[query].kperfmon_ids[i]);
+ if (!perfmon) {
+ DRM_DEBUG("Failed to find perfmon.");
+ continue;
+ }
+
+ v3d_perfmon_stop(v3d, perfmon, true);
+
+ memcpy(_values[i * DRM_V3D_MAX_PERF_COUNTERS],
perfmon->values,
+ perfmon->ncounters * sizeof(u64));
+
+ v3d_perfmon_put(perfmon);
+ }
+
+ for (int i = 0; i < performance_query->ncounters; i++)
+ write_to_buffer(data, i, copy->do_64bit, counter_values[i]);
+}
+
+
+static void
+v3d_copy_performance_query(struct v3d_cpu_job *job)
+{
+ struct v3d_performance_query_info *performance_query =
>performance_query;
+ struct v3d_copy_query_results_info *copy = >copy;
+ struct v3d_bo *bo = to_v3d_bo(job->base.bo[0]);
+ struct dma_fence *fence;
+ bool available, write_result;
+ u8 *data;
+
+ v3d_get_bo_vaddr(bo);
+
+ data = ((u8 *) bo->vaddr) + copy->offset;
+
+ for (int i = 0; i < performance_query->count; i++) {
+ fence =
drm_syncobj_fence_get(performance_query->queries[i].syncobj);
+ available = fence ? dma_fence_is_signaled(fence) : false;
+
+ write_result = available || copy->do_partial;
+ if (write_result)
+ v3d_write_performance_query_result(job, data, i);
+
+ if (copy->availability_bit)
+ write_to_buffer(data, performance_query->ncounters,
+ copy->do_64bit, available ? 1u : 0u);
+
+ data += copy->stride;
+
+ dma_fence_put(fence);
+ }
+
+ v3d_put_bo_vaddr(bo);
+}
+
static const v3d_cpu_job_fn cpu_job_function[] = {
[V3D_CPU_JOB_TYPE_INDIRECT_CSD] =
v3d_rewrite_csd_job_wg_counts_from_indirect,
[V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY] = v3d_timestamp_query,
[V3D_CPU_JOB_TYPE_RESET_TIMESTAMP_QUERY] = v3d_reset_timestamp_queries,
[V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY] = v3d_copy_query_results,
[V3D_CPU_JOB_TYPE_RESET_PERFORMANCE_QUERY] =
v3d_reset_performance_queries,
+ [V3D_CPU_JOB_TYPE_COPY_PERFORMANCE_QUERY] = v3d_copy_performance_query,
};
static struct dma_fence *
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 20af8ae14831..d7a9da2484fd 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -672,6 +672,84 @@ v3d_get_cpu_reset_performance_params(struct drm_file
*file_priv,
return 0;
}
+static int
+v3d_get_cpu_copy_performance_query_params(struct drm_file *file_priv,
+
[PATCH v3 13/17] drm/v3d: Create a CPU job extension for the timestamp query job
A CPU job is a type of job that performs operations that requires CPU
intervention. A timestamp query job is a job that calculates the
query timestamp and updates the query availability by signaling a
syncobj. As V3D doesn't provide any mechanism to obtain a timestamp
from the GPU, it is a job that needs CPU intervention.
So, create a user extension for the CPU job that enables the creation
of a timestamp query job. This user extension will allow the creation of
a CPU job that performs the timestamp query calculation and updates the
timestamp BO with the proper value.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.h| 17 +
drivers/gpu/drm/v3d/v3d_sched.c | 40 +++-
drivers/gpu/drm/v3d/v3d_submit.c | 63
include/uapi/drm/v3d_drm.h | 30 +++
4 files changed, 149 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 202c0d4b04a5..dd86e745c260 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -318,6 +318,15 @@ struct v3d_csd_job {
enum v3d_cpu_job_type {
V3D_CPU_JOB_TYPE_INDIRECT_CSD = 1,
+ V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY,
+};
+
+struct v3d_timestamp_query {
+ /* Offset of this query in the timestamp BO for its value. */
+ u32 offset;
+
+ /* Syncobj that indicates the timestamp availability */
+ struct drm_syncobj *syncobj;
};
struct v3d_indirect_csd_info {
@@ -345,12 +354,20 @@ struct v3d_indirect_csd_info {
struct ww_acquire_ctx acquire_ctx;
};
+struct v3d_timestamp_query_info {
+ struct v3d_timestamp_query *queries;
+
+ u32 count;
+};
+
struct v3d_cpu_job {
struct v3d_job base;
enum v3d_cpu_job_type job_type;
struct v3d_indirect_csd_info indirect_csd;
+
+ struct v3d_timestamp_query_info timestamp_query;
};
typedef void (*v3d_cpu_job_fn)(struct v3d_cpu_job *);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 597e4ec3d28d..f4cffefb6398 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -21,6 +21,8 @@
#include
#include
+#include
+
#include "v3d_drv.h"
#include "v3d_regs.h"
#include "v3d_trace.h"
@@ -71,6 +73,21 @@ v3d_sched_job_free(struct drm_sched_job *sched_job)
v3d_job_cleanup(job);
}
+static void
+v3d_cpu_job_free(struct drm_sched_job *sched_job)
+{
+ struct v3d_cpu_job *job = to_cpu_job(sched_job);
+ struct v3d_timestamp_query_info *timestamp_query =
>timestamp_query;
+
+ if (timestamp_query->queries) {
+ for (int i = 0; i < timestamp_query->count; i++)
+ drm_syncobj_put(timestamp_query->queries[i].syncobj);
+ kvfree(timestamp_query->queries);
+ }
+
+ v3d_job_cleanup(>base);
+}
+
static void
v3d_switch_perfmon(struct v3d_dev *v3d, struct v3d_job *job)
{
@@ -305,8 +322,29 @@ v3d_rewrite_csd_job_wg_counts_from_indirect(struct
v3d_cpu_job *job)
v3d_put_bo_vaddr(bo);
}
+static void
+v3d_timestamp_query(struct v3d_cpu_job *job)
+{
+ struct v3d_timestamp_query_info *timestamp_query =
>timestamp_query;
+ struct v3d_bo *bo = to_v3d_bo(job->base.bo[0]);
+ u8 *value_addr;
+
+ v3d_get_bo_vaddr(bo);
+
+ for (int i = 0; i < timestamp_query->count; i++) {
+ value_addr = ((u8 *) bo->vaddr) +
timestamp_query->queries[i].offset;
+ *((u64 *) value_addr) = i == 0 ? ktime_get_ns() : 0ull;
+
+ drm_syncobj_replace_fence(timestamp_query->queries[i].syncobj,
+ job->base.done_fence);
+ }
+
+ v3d_put_bo_vaddr(bo);
+}
+
static const v3d_cpu_job_fn cpu_job_function[] = {
[V3D_CPU_JOB_TYPE_INDIRECT_CSD] =
v3d_rewrite_csd_job_wg_counts_from_indirect,
+ [V3D_CPU_JOB_TYPE_TIMESTAMP_QUERY] = v3d_timestamp_query,
};
static struct dma_fence *
@@ -504,7 +542,7 @@ static const struct drm_sched_backend_ops
v3d_cache_clean_sched_ops = {
static const struct drm_sched_backend_ops v3d_cpu_sched_ops = {
.run_job = v3d_cpu_job_run,
.timedout_job = v3d_generic_job_timedout,
- .free_job = v3d_sched_job_free
+ .free_job = v3d_cpu_job_free
};
int
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 0320695b941b..83e029e786ea 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -433,6 +433,64 @@ v3d_get_cpu_indirect_csd_params(struct drm_file *file_priv,
NULL, >acquire_ctx);
}
+/* Get data for the query timestamp job submission. */
+static int
+v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv,
+ struct drm_v3d_extension __user *ext,
+ struct v3d_cpu_job *job)
+{
+ u32 __user *offsets, *syncs;
+ struct
[PATCH v3 12/17] drm/v3d: Create a CPU job extension for a indirect CSD job
A CPU job is a type of job that performs operations that requires CPU
intervention. An indirect CSD job is a job that, when executed in the
queue, will map the indirect buffer, read the dispatch parameters, and
submit a regular dispatch. Therefore, it is a job that needs CPU
intervention.
So, create a user extension for the CPU job that enables the creation
of an indirect CSD. This user extension will allow the creation of a CSD
job linked to a CPU job. The CPU job will wait for the indirect CSD job
dependencies and, once they are signaled, it will update the CSD job
parameters.
Co-developed-by: Melissa Wen
Signed-off-by: Melissa Wen
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.h| 31 -
drivers/gpu/drm/v3d/v3d_sched.c | 41 +++-
drivers/gpu/drm/v3d/v3d_submit.c | 104 ++-
include/uapi/drm/v3d_drm.h | 43 -
4 files changed, 213 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 39d62915cdd6..202c0d4b04a5 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -316,12 +316,41 @@ struct v3d_csd_job {
struct drm_v3d_submit_csd args;
};
-enum v3d_cpu_job_type {};
+enum v3d_cpu_job_type {
+ V3D_CPU_JOB_TYPE_INDIRECT_CSD = 1,
+};
+
+struct v3d_indirect_csd_info {
+ /* Indirect CSD */
+ struct v3d_csd_job *job;
+
+ /* Clean cache job associated to the Indirect CSD job */
+ struct v3d_job *clean_job;
+
+ /* Offset within the BO where the workgroup counts are stored */
+ u32 offset;
+
+ /* Workgroups size */
+ u32 wg_size;
+
+ /* Indices of the uniforms with the workgroup dispatch counts
+* in the uniform stream.
+*/
+ u32 wg_uniform_offsets[3];
+
+ /* Indirect BO */
+ struct drm_gem_object *indirect;
+
+ /* Context of the Indirect CSD job */
+ struct ww_acquire_ctx acquire_ctx;
+};
struct v3d_cpu_job {
struct v3d_job base;
enum v3d_cpu_job_type job_type;
+
+ struct v3d_indirect_csd_info indirect_csd;
};
typedef void (*v3d_cpu_job_fn)(struct v3d_cpu_job *);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index ebbd00840a73..597e4ec3d28d 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -25,6 +25,8 @@
#include "v3d_regs.h"
#include "v3d_trace.h"
+#define V3D_CSD_CFG012_WG_COUNT_SHIFT 16
+
static struct v3d_job *
to_v3d_job(struct drm_sched_job *sched_job)
{
@@ -268,7 +270,44 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
return fence;
}
-static const v3d_cpu_job_fn cpu_job_function[] = { };
+static void
+v3d_rewrite_csd_job_wg_counts_from_indirect(struct v3d_cpu_job *job)
+{
+ struct v3d_indirect_csd_info *indirect_csd = >indirect_csd;
+ struct v3d_bo *bo = to_v3d_bo(job->base.bo[0]);
+ struct v3d_bo *indirect = to_v3d_bo(indirect_csd->indirect);
+ struct drm_v3d_submit_csd *args = _csd->job->args;
+ u32 *wg_counts;
+
+ v3d_get_bo_vaddr(bo);
+ v3d_get_bo_vaddr(indirect);
+
+ wg_counts = (uint32_t *) (bo->vaddr + indirect_csd->offset);
+
+ if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
+ return;
+
+ args->cfg[0] = wg_counts[0] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
+ args->cfg[1] = wg_counts[1] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
+ args->cfg[2] = wg_counts[2] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
+ args->cfg[4] = DIV_ROUND_UP(indirect_csd->wg_size, 16) *
+ (wg_counts[0] * wg_counts[1] * wg_counts[2]) - 1;
+
+ for (int i = 0; i < 3; i++) {
+ /* 0x indicates that the uniform rewrite is not needed
*/
+ if (indirect_csd->wg_uniform_offsets[i] != 0x) {
+ u32 uniform_idx = indirect_csd->wg_uniform_offsets[i];
+ ((uint32_t *) indirect->vaddr)[uniform_idx] =
wg_counts[i];
+ }
+ }
+
+ v3d_put_bo_vaddr(indirect);
+ v3d_put_bo_vaddr(bo);
+}
+
+static const v3d_cpu_job_fn cpu_job_function[] = {
+ [V3D_CPU_JOB_TYPE_INDIRECT_CSD] =
v3d_rewrite_csd_job_wg_counts_from_indirect,
+};
static struct dma_fence *
v3d_cpu_job_run(struct drm_sched_job *sched_job)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index eb26fe1e27e3..0320695b941b 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -391,6 +391,48 @@ v3d_get_multisync_submit_deps(struct drm_file *file_priv,
return 0;
}
+/* Get data for the indirect CSD job submission. */
+static int
+v3d_get_cpu_indirect_csd_params(struct drm_file *file_priv,
+ struct drm_v3d_extension __user *ext,
+ struct v3d_cpu_job *job)
+{
+ struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
+
[PATCH v3 10/17] drm/v3d: Detach the CSD job BO setup
From: Melissa Wen
Detach CSD job setup from CSD submission ioctl to reuse it in CPU
submission ioctl for indirect CSD job.
Signed-off-by: Melissa Wen
Co-developed-by: Maíra Canal
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 68
1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index c134b113b181..eb26fe1e27e3 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -256,6 +256,45 @@ v3d_attach_fences_and_unlock_reservation(struct drm_file
*file_priv,
}
}
+static int
+v3d_setup_csd_jobs_and_bos(struct drm_file *file_priv,
+ struct v3d_dev *v3d,
+ struct drm_v3d_submit_csd *args,
+ struct v3d_csd_job **job,
+ struct v3d_job **clean_job,
+ struct v3d_submit_ext *se,
+ struct ww_acquire_ctx *acquire_ctx)
+{
+ int ret;
+
+ ret = v3d_job_allocate((void *)job, sizeof(**job));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, &(*job)->base,
+ v3d_job_free, args->in_sync, se, V3D_CSD);
+ if (ret)
+ return ret;
+
+ ret = v3d_job_allocate((void *)clean_job, sizeof(**clean_job));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, *clean_job,
+ v3d_job_free, 0, NULL, V3D_CACHE_CLEAN);
+ if (ret)
+ return ret;
+
+ (*job)->args = *args;
+
+ ret = v3d_lookup_bos(>drm, file_priv, *clean_job,
+args->bo_handles, args->bo_handle_count);
+ if (ret)
+ return ret;
+
+ return v3d_lock_bo_reservations(*clean_job, acquire_ctx);
+}
+
static void
v3d_put_multisync_post_deps(struct v3d_submit_ext *se)
{
@@ -700,32 +739,9 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
}
}
- ret = v3d_job_allocate((void *), sizeof(*job));
- if (ret)
- return ret;
-
- ret = v3d_job_init(v3d, file_priv, >base,
- v3d_job_free, args->in_sync, , V3D_CSD);
- if (ret)
- goto fail;
-
- ret = v3d_job_allocate((void *)_job, sizeof(*clean_job));
- if (ret)
- goto fail;
-
- ret = v3d_job_init(v3d, file_priv, clean_job,
- v3d_job_free, 0, NULL, V3D_CACHE_CLEAN);
- if (ret)
- goto fail;
-
- job->args = *args;
-
- ret = v3d_lookup_bos(dev, file_priv, clean_job,
-args->bo_handles, args->bo_handle_count);
- if (ret)
- goto fail;
-
- ret = v3d_lock_bo_reservations(clean_job, _ctx);
+ ret = v3d_setup_csd_jobs_and_bos(file_priv, v3d, args,
+, _job, ,
+_ctx);
if (ret)
goto fail;
--
2.42.0
[PATCH v3 11/17] drm/v3d: Enable BO mapping
For the indirect CSD CPU job, we will need to access the internal
contents of the BO with the dispatch parameters. Therefore, create
methods to allow the mapping and unmapping of the BO.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_bo.c | 18 ++
drivers/gpu/drm/v3d/v3d_drv.h | 4
2 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 357a0da7e16a..1bdfac8beafd 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -33,6 +33,9 @@ void v3d_free_object(struct drm_gem_object *obj)
struct v3d_dev *v3d = to_v3d_dev(obj->dev);
struct v3d_bo *bo = to_v3d_bo(obj);
+ if (bo->vaddr)
+ v3d_put_bo_vaddr(bo);
+
v3d_mmu_remove_ptes(bo);
mutex_lock(>bo_lock);
@@ -134,6 +137,7 @@ struct v3d_bo *v3d_bo_create(struct drm_device *dev, struct
drm_file *file_priv,
if (IS_ERR(shmem_obj))
return ERR_CAST(shmem_obj);
bo = to_v3d_bo(_obj->base);
+ bo->vaddr = NULL;
ret = v3d_bo_create_finish(_obj->base);
if (ret)
@@ -167,6 +171,20 @@ v3d_prime_import_sg_table(struct drm_device *dev,
return obj;
}
+void v3d_get_bo_vaddr(struct v3d_bo *bo)
+{
+ struct drm_gem_shmem_object *obj = >base;
+
+ bo->vaddr = vmap(obj->pages, obj->base.size >> PAGE_SHIFT, VM_MAP,
+pgprot_writecombine(PAGE_KERNEL));
+}
+
+void v3d_put_bo_vaddr(struct v3d_bo *bo)
+{
+ vunmap(bo->vaddr);
+ bo->vaddr = NULL;
+}
+
int v3d_create_bo_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 2246a0e29955..39d62915cdd6 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -202,6 +202,8 @@ struct v3d_bo {
* v3d_render_job->unref_list
*/
struct list_head unref_head;
+
+ void *vaddr;
};
static inline struct v3d_bo *
@@ -391,6 +393,8 @@ struct drm_gem_object *v3d_create_object(struct drm_device
*dev, size_t size);
void v3d_free_object(struct drm_gem_object *gem_obj);
struct v3d_bo *v3d_bo_create(struct drm_device *dev, struct drm_file
*file_priv,
size_t size);
+void v3d_get_bo_vaddr(struct v3d_bo *bo);
+void v3d_put_bo_vaddr(struct v3d_bo *bo);
int v3d_create_bo_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int v3d_mmap_bo_ioctl(struct drm_device *dev, void *data,
--
2.42.0
[PATCH v3 09/17] drm/v3d: Create tracepoints to track the CPU job
Create tracepoints to track the three major events of a CPU job
lifetime:
1. Submission of a `v3d_submit_cpu` IOCTL
2. Beginning of the execution of a CPU job
3. Ending of the execution of a CPU job
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_sched.c | 4 +++
drivers/gpu/drm/v3d/v3d_submit.c | 2 ++
drivers/gpu/drm/v3d/v3d_trace.h | 57
3 files changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index c89e92fc614c..ebbd00840a73 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -288,8 +288,12 @@ v3d_cpu_job_run(struct drm_sched_job *sched_job)
file->start_ns[V3D_CPU] = local_clock();
v3d->queue[V3D_CPU].start_ns = file->start_ns[V3D_CPU];
+ trace_v3d_cpu_job_begin(>drm, job->job_type);
+
cpu_job_function[job->job_type](job);
+ trace_v3d_cpu_job_end(>drm, job->job_type);
+
runtime = local_clock() - file->start_ns[V3D_CPU];
file->enabled_ns[V3D_CPU] += runtime;
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 124935547f17..c134b113b181 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -824,6 +824,8 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
goto fail;
}
+ trace_v3d_submit_cpu_ioctl(>drm, cpu_job->job_type);
+
ret = v3d_job_init(v3d, file_priv, _job->base,
v3d_job_free, 0, , V3D_CPU);
if (ret)
diff --git a/drivers/gpu/drm/v3d/v3d_trace.h b/drivers/gpu/drm/v3d/v3d_trace.h
index 7aa8dc356e54..06086ece6e9e 100644
--- a/drivers/gpu/drm/v3d/v3d_trace.h
+++ b/drivers/gpu/drm/v3d/v3d_trace.h
@@ -225,6 +225,63 @@ TRACE_EVENT(v3d_submit_csd,
__entry->seqno)
);
+TRACE_EVENT(v3d_submit_cpu_ioctl,
+ TP_PROTO(struct drm_device *dev, enum v3d_cpu_job_type job_type),
+ TP_ARGS(dev, job_type),
+
+ TP_STRUCT__entry(
+ __field(u32, dev)
+ __field(enum v3d_cpu_job_type, job_type)
+ ),
+
+ TP_fast_assign(
+ __entry->dev = dev->primary->index;
+ __entry->job_type = job_type;
+ ),
+
+ TP_printk("dev=%u, job_type=%d",
+__entry->dev,
+__entry->job_type)
+);
+
+TRACE_EVENT(v3d_cpu_job_begin,
+ TP_PROTO(struct drm_device *dev, enum v3d_cpu_job_type job_type),
+ TP_ARGS(dev, job_type),
+
+ TP_STRUCT__entry(
+__field(u32, dev)
+__field(enum v3d_cpu_job_type, job_type)
+),
+
+ TP_fast_assign(
+ __entry->dev = dev->primary->index;
+ __entry->job_type = job_type;
+ ),
+
+ TP_printk("dev=%u, job_type=%d",
+ __entry->dev,
+ __entry->job_type)
+);
+
+TRACE_EVENT(v3d_cpu_job_end,
+ TP_PROTO(struct drm_device *dev, enum v3d_cpu_job_type job_type),
+ TP_ARGS(dev, job_type),
+
+ TP_STRUCT__entry(
+__field(u32, dev)
+__field(enum v3d_cpu_job_type, job_type)
+),
+
+ TP_fast_assign(
+ __entry->dev = dev->primary->index;
+ __entry->job_type = job_type;
+ ),
+
+ TP_printk("dev=%u, job_type=%d",
+ __entry->dev,
+ __entry->job_type)
+);
+
TRACE_EVENT(v3d_cache_clean_begin,
TP_PROTO(struct drm_device *dev),
TP_ARGS(dev),
--
2.42.0
[PATCH v3 07/17] drm/v3d: Add a CPU job submission
From: Melissa Wen
Create a new type of job, a CPU job. A CPU job is a type of job that
performs operations that requires CPU intervention. The overall idea is
to use user extensions to enable different types of CPU job, allowing the
CPU job to perform different operations according to the type of user
extension. The user extension ID identify the type of CPU job that must
be dealt.
Having a CPU job is interesting for synchronization purposes as a CPU
job has a queue like any other V3D job and can be synchoronized by the
multisync extension.
Signed-off-by: Melissa Wen
Co-developed-by: Maíra Canal
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_drv.c| 4 ++
drivers/gpu/drm/v3d/v3d_drv.h| 16 +-
drivers/gpu/drm/v3d/v3d_sched.c | 57 +
drivers/gpu/drm/v3d/v3d_submit.c | 86
include/uapi/drm/v3d_drm.h | 17 +++
5 files changed, 179 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 44a1ca57d6a4..3debf37e7d9b 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -91,6 +91,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void
*data,
case DRM_V3D_PARAM_SUPPORTS_MULTISYNC_EXT:
args->value = 1;
return 0;
+ case DRM_V3D_PARAM_SUPPORTS_CPU_QUEUE:
+ args->value = 1;
+ return 0;
default:
DRM_DEBUG("Unknown parameter %d\n", args->param);
return -EINVAL;
@@ -189,6 +192,7 @@ static const struct drm_ioctl_desc v3d_drm_ioctls[] = {
DRM_IOCTL_DEF_DRV(V3D_PERFMON_CREATE, v3d_perfmon_create_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(V3D_PERFMON_DESTROY, v3d_perfmon_destroy_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(V3D_PERFMON_GET_VALUES, v3d_perfmon_get_values_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(V3D_SUBMIT_CPU, v3d_submit_cpu_ioctl,
DRM_RENDER_ALLOW | DRM_AUTH),
};
static const struct drm_driver v3d_drm_driver = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 4db9ace66024..2246a0e29955 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -19,7 +19,7 @@ struct reset_control;
#define GMP_GRANULARITY (128 * 1024)
-#define V3D_MAX_QUEUES (V3D_CACHE_CLEAN + 1)
+#define V3D_MAX_QUEUES (V3D_CPU + 1)
static inline char *v3d_queue_to_string(enum v3d_queue queue)
{
@@ -29,6 +29,7 @@ static inline char *v3d_queue_to_string(enum v3d_queue queue)
case V3D_TFU: return "tfu";
case V3D_CSD: return "csd";
case V3D_CACHE_CLEAN: return "cache_clean";
+ case V3D_CPU: return "cpu";
}
return "UNKNOWN";
}
@@ -122,6 +123,7 @@ struct v3d_dev {
struct v3d_render_job *render_job;
struct v3d_tfu_job *tfu_job;
struct v3d_csd_job *csd_job;
+ struct v3d_cpu_job *cpu_job;
struct v3d_queue_state queue[V3D_MAX_QUEUES];
@@ -312,6 +314,16 @@ struct v3d_csd_job {
struct drm_v3d_submit_csd args;
};
+enum v3d_cpu_job_type {};
+
+struct v3d_cpu_job {
+ struct v3d_job base;
+
+ enum v3d_cpu_job_type job_type;
+};
+
+typedef void (*v3d_cpu_job_fn)(struct v3d_cpu_job *);
+
struct v3d_submit_outsync {
struct drm_syncobj *syncobj;
};
@@ -414,6 +426,8 @@ int v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+int v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_priv);
/* v3d_irq.c */
int v3d_irq_init(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index fccbea2a5f2e..c89e92fc614c 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -55,6 +55,12 @@ to_csd_job(struct drm_sched_job *sched_job)
return container_of(sched_job, struct v3d_csd_job, base.base);
}
+static struct v3d_cpu_job *
+to_cpu_job(struct drm_sched_job *sched_job)
+{
+ return container_of(sched_job, struct v3d_cpu_job, base.base);
+}
+
static void
v3d_sched_job_free(struct drm_sched_job *sched_job)
{
@@ -262,6 +268,42 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
return fence;
}
+static const v3d_cpu_job_fn cpu_job_function[] = { };
+
+static struct dma_fence *
+v3d_cpu_job_run(struct drm_sched_job *sched_job)
+{
+ struct v3d_cpu_job *job = to_cpu_job(sched_job);
+ struct v3d_dev *v3d = job->base.v3d;
+ struct v3d_file_priv *file = job->base.file->driver_priv;
+ u64 runtime;
+
+ v3d->cpu_job = job;
+
+ if (job->job_type >= ARRAY_SIZE(cpu_job_function)) {
+ DRM_DEBUG_DRIVER("Unknown CPU job: %d\n", job->job_type);
+ return NULL;
+ }
+
+
[PATCH v3 08/17] drm/v3d: Use v3d_get_extensions() to parse CPU job data
Currently, v3d_get_extensions() only parses multisync data and assigns
it to the `struct v3d_submit_ext`. But, to implement the CPU job with
user extensions, we want v3d_get_extensions() to be able to parse CPU
job data and assign it to the `struct v3d_cpu_job`.
Therefore, allow the function v3d_get_extensions() to use `struct v3d_cpu_job *`
as a parameter. If the `struct v3d_cpu_job *` is assigned to NULL, it means
that the job is a GPU job and CPU job extensions should be rejected.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 918e28b1d00a..124935547f17 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -323,10 +323,9 @@ v3d_get_multisync_post_deps(struct drm_file *file_priv,
static int
v3d_get_multisync_submit_deps(struct drm_file *file_priv,
struct drm_v3d_extension __user *ext,
- void *data)
+ struct v3d_submit_ext *se)
{
struct drm_v3d_multi_sync multisync;
- struct v3d_submit_ext *se = data;
int ret;
if (se->in_sync_count || se->out_sync_count) {
@@ -340,7 +339,7 @@ v3d_get_multisync_submit_deps(struct drm_file *file_priv,
if (multisync.pad)
return -EINVAL;
- ret = v3d_get_multisync_post_deps(file_priv, data,
multisync.out_sync_count,
+ ret = v3d_get_multisync_post_deps(file_priv, se,
multisync.out_sync_count,
multisync.out_syncs);
if (ret)
return ret;
@@ -359,7 +358,8 @@ v3d_get_multisync_submit_deps(struct drm_file *file_priv,
static int
v3d_get_extensions(struct drm_file *file_priv,
u64 ext_handles,
- void *data)
+ struct v3d_submit_ext *se,
+ struct v3d_cpu_job *job)
{
struct drm_v3d_extension __user *user_ext;
int ret;
@@ -375,15 +375,16 @@ v3d_get_extensions(struct drm_file *file_priv,
switch (ext.id) {
case DRM_V3D_EXT_ID_MULTI_SYNC:
- ret = v3d_get_multisync_submit_deps(file_priv,
user_ext, data);
- if (ret)
- return ret;
+ ret = v3d_get_multisync_submit_deps(file_priv,
user_ext, se);
break;
default:
DRM_DEBUG_DRIVER("Unknown extension id: %d\n", ext.id);
return -EINVAL;
}
+ if (ret)
+ return ret;
+
user_ext = u64_to_user_ptr(ext.next);
}
@@ -430,7 +431,7 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
}
if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
- ret = v3d_get_extensions(file_priv, args->extensions, );
+ ret = v3d_get_extensions(file_priv, args->extensions, ,
NULL);
if (ret) {
DRM_DEBUG("Failed to get extensions.\n");
return ret;
@@ -585,7 +586,7 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
}
if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
- ret = v3d_get_extensions(file_priv, args->extensions, );
+ ret = v3d_get_extensions(file_priv, args->extensions, ,
NULL);
if (ret) {
DRM_DEBUG("Failed to get extensions.\n");
return ret;
@@ -692,7 +693,7 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
}
if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
- ret = v3d_get_extensions(file_priv, args->extensions, );
+ ret = v3d_get_extensions(file_priv, args->extensions, ,
NULL);
if (ret) {
DRM_DEBUG("Failed to get extensions.\n");
return ret;
@@ -805,7 +806,7 @@ v3d_submit_cpu_ioctl(struct drm_device *dev, void *data,
return ret;
if (args->flags & DRM_V3D_SUBMIT_EXTENSION) {
- ret = v3d_get_extensions(file_priv, args->extensions, );
+ ret = v3d_get_extensions(file_priv, args->extensions, ,
cpu_job);
if (ret) {
DRM_DEBUG("Failed to get extensions.\n");
goto fail;
--
2.42.0
[PATCH v3 06/17] drm/v3d: Decouple job allocation from job initiation
We want to allow the IOCTLs to allocate the job without initiating it.
This will be useful for the CPU job submission IOCTL, as the CPU job has
the need to use information from the user extensions. Currently, the
user extensions are parsed before the job allocation, making it
impossible to fill the CPU job when parsing the user extensions.
Therefore, decouple the job allocation from the job initiation.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 64 ++--
1 file changed, 44 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index 10141dc2820a..148900283c2a 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -135,23 +135,27 @@ void v3d_job_put(struct v3d_job *job)
kref_put(>refcount, job->free);
}
+static int
+v3d_job_allocate(void **container, size_t size)
+{
+ *container = kcalloc(1, size, GFP_KERNEL);
+ if (!*container) {
+ DRM_ERROR("Cannot allocate memory for V3D job.\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int
v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
-void **container, size_t size, void (*free)(struct kref *ref),
+struct v3d_job *job, void (*free)(struct kref *ref),
u32 in_sync, struct v3d_submit_ext *se, enum v3d_queue queue)
{
struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
- struct v3d_job *job;
bool has_multisync = se && (se->flags & DRM_V3D_EXT_ID_MULTI_SYNC);
int ret, i;
- *container = kcalloc(1, size, GFP_KERNEL);
- if (!*container) {
- DRM_ERROR("Cannot allocate memory for v3d job.");
- return -ENOMEM;
- }
-
- job = *container;
job->v3d = v3d;
job->free = free;
job->file = file_priv;
@@ -159,7 +163,7 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file
*file_priv,
ret = drm_sched_job_init(>base, _priv->sched_entity[queue],
1, v3d_priv);
if (ret)
- goto fail;
+ return ret;
if (has_multisync) {
if (se->in_sync_count && se->wait_stage == queue) {
@@ -194,10 +198,6 @@ v3d_job_init(struct v3d_dev *v3d, struct drm_file
*file_priv,
fail_deps:
drm_sched_job_cleanup(>base);
-fail:
- kfree(*container);
- *container = NULL;
-
return ret;
}
@@ -437,7 +437,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
}
}
- ret = v3d_job_init(v3d, file_priv, (void *), sizeof(*render),
+ ret = v3d_job_allocate((void *), sizeof(*render));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, >base,
v3d_render_job_free, args->in_sync_rcl, ,
V3D_RENDER);
if (ret)
goto fail;
@@ -447,7 +451,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
INIT_LIST_HEAD(>unref_list);
if (args->bcl_start != args->bcl_end) {
- ret = v3d_job_init(v3d, file_priv, (void *), sizeof(*bin),
+ ret = v3d_job_allocate((void *), sizeof(*bin));
+ if (ret)
+ goto fail;
+
+ ret = v3d_job_init(v3d, file_priv, >base,
v3d_job_free, args->in_sync_bcl, ,
V3D_BIN);
if (ret)
goto fail;
@@ -461,7 +469,11 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
}
if (args->flags & DRM_V3D_SUBMIT_CL_FLUSH_CACHE) {
- ret = v3d_job_init(v3d, file_priv, (void *)_job,
sizeof(*clean_job),
+ ret = v3d_job_allocate((void *)_job, sizeof(*clean_job));
+ if (ret)
+ goto fail;
+
+ ret = v3d_job_init(v3d, file_priv, clean_job,
v3d_job_free, 0, NULL, V3D_CACHE_CLEAN);
if (ret)
goto fail;
@@ -580,7 +592,11 @@ v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
}
}
- ret = v3d_job_init(v3d, file_priv, (void *), sizeof(*job),
+ ret = v3d_job_allocate((void *), sizeof(*job));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, >base,
v3d_job_free, args->in_sync, , V3D_TFU);
if (ret)
goto fail;
@@ -683,12 +699,20 @@ v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
}
}
- ret = v3d_job_init(v3d, file_priv, (void *), sizeof(*job),
+ ret = v3d_job_allocate((void *), sizeof(*job));
+ if (ret)
+ return ret;
+
+ ret = v3d_job_init(v3d, file_priv, >base,
v3d_job_free, args->in_sync, , V3D_CSD);
if (ret)
goto fail;
-
[PATCH v3 00/17] drm/v3d: Introduce CPU jobs
This patchset implements the basic infrastructure for a new type of V3D job, a CPU job. A CPU job is a job that requires CPU intervention. It would be nice to perform this operations on the kernel space as we can attach multiple in/out syncobjs to it. Why we want a CPU job on the kernel? There are some Vulkan commands that cannot be performed by the GPU, so we implement those as CPU jobs on Mesa. But to synchronize a CPU job in the user space, we need to hold part of the command submission flow in order to correctly synchronize their execution. By moving the CPU job to the kernel, we can make use of the DRM schedule queues and all the advantages it brings with it. This way, instead of stalling the submission thread, we can use syncobjs to synchronize the job, providing a more effective management. About the implementation After we decided that we would like to have a CPU job implementation in the kernel, we could think about two possible implementations for this job: creating an IOCTL for each type of CPU job or using an user extension to provide a polymorphic behavior to a single CPU job IOCTL. We decided for the latter one. We have different types of CPU jobs (indirect CSD jobs, timestamp query jobs, copy query results jobs...) and each of them have a common infrastructure, but perform different operations. Therefore, by using a single IOCTL that is extended by an user extension, we can reuse the common infrastructure - avoiding code repetition - and yet use the user extension ID to identify the type of job and depending on the type of job, perform a certain operation. About the patchset == This patchset introduces the basic infrastructure of a CPU job with a new V3D queue (V3D_CPU) e new tracers. Moreover, it introduces six types of CPU jobs: an indirect CSD job, a timestamp query job, a reset timestamp queries job, a copy timestamp query results job, a reset performance queries job, and a copy performance query results job. An indirect CSD job is a job that, when executed in the queue, will map the indirect buffer, read the dispatch parameters, and submit a regular dispatch. So, the CSD job depends on the CPU job execution. We attach the wait dependencies to the CPU job and once they are satisfied, we read the dispatch parameters, rewrite the uniforms (if needed) and enable the CSD job execution, which depends on the completion of the CPU job. A timestamp query job is a job that calculates the value of the timestamp query and updates the availability of the query. In order to implement this job, we had to change the Mesa implementation of the timestamp. Now, the timestamp query value is tracked in a BO, instead of using a memory address. Moreover, the timestamp query availability is tracked with a syncobj, which is signaled when the query is available. A reset timestamp queries job is a job that resets the timestamp queries by zeroing the timestamp BO in the right positions. The right position on the timestamp BO is found through the offset of the first query. A reset performance queries job is a job that zeros the values of the performance monitors associated to that query. Moreover, it resets the availability syncobj related to that query. A copy query results job is a job that copy the results of a query to a BO in a given offset with a given stride. The patchset is divided as such: * #1 - #4: refactoring operations to prepare for the introduction of the CPU job * #5: addressing a vulnerability in the multisync extension * #6: decouple job allocation from job initiation * #7 - #9: introduction of the CPU job * #10 - #11: refactoring operations to prepare for the introduction of the indirect CSD job * #12: introduction of the indirect CSD job * #13: introduction of the timestamp query job * #14: introduction of the reset timestamp queries job * #15: introduction of the copy timestamp query results job * #16: introduction of the reset performance queries job * #17: introduction of the copy performance query results job This patchset has its Mesa counterpart, which is available on [1]. Both the kernel and Mesa implementation were tested with * `dEQP-VK.compute.pipeline.indirect_dispatch.*`, * `dEQP-VK.pipeline.monolithic.timestamp.*`, * `dEQP-VK.synchronization.*`, * `dEQP-VK.query_pool.*` * and `dEQP-VK.multiview.*`. [1] https://gitlab.freedesktop.org/mairacanal/mesa/-/tree/v3dv/v5/cpu-job Changelog = v1 -> v2: https://lore.kernel.org/dri-devel/20230904175019.1172713-1-mca...@igalia.com/ * Rebase on top of drm-misc-next. * Add GPU stats to the CPU queue. v2 -> v3: https://lore.kernel.org/dri-devel/20231124012548.772095-1-mca...@igalia.com/ * Don't cast struct v3d_*_job to void *, use >base (Iago Toral) * Completely decouple job allocation from initialization (Iago Toral & Melissa Wen) * s/externsion/extension (Iago Toral) * Declare all CPU job
[PATCH v3 05/17] drm/v3d: Don't allow two multisync extensions in the same job
Currently, two multisync extensions can be added to the same job and
only the last multisync extension will be used. To avoid this
vulnerability, don't allow two multisync extensions in the same job.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index a0caf9c499bb..10141dc2820a 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -329,6 +329,11 @@ v3d_get_multisync_submit_deps(struct drm_file *file_priv,
struct v3d_submit_ext *se = data;
int ret;
+ if (se->in_sync_count || se->out_sync_count) {
+ DRM_DEBUG("Two multisync extensions were added to the same
job.");
+ return -EINVAL;
+ }
+
if (copy_from_user(, ext, sizeof(multisync)))
return -EFAULT;
--
2.42.0
[PATCH v3 03/17] drm/v3d: Detach job submissions IOCTLs to a new specific file
From: Melissa Wen
We will include a new job submission type, the CPU job submission. For
readability and maintability, separate the job submission IOCTLs and
related operations from v3d_gem.c.
Minor fix in the CSD submission kernel doc:
CSD (texture formatting) -> CSD (compute shader).
Signed-off-by: Melissa Wen
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/Makefile | 3 +-
drivers/gpu/drm/v3d/v3d_drv.h| 12 +-
drivers/gpu/drm/v3d/v3d_gem.c| 735 --
drivers/gpu/drm/v3d/v3d_submit.c | 744 +++
4 files changed, 753 insertions(+), 741 deletions(-)
create mode 100644 drivers/gpu/drm/v3d/v3d_submit.c
diff --git a/drivers/gpu/drm/v3d/Makefile b/drivers/gpu/drm/v3d/Makefile
index 4b21b20e4998..b7d673f1153b 100644
--- a/drivers/gpu/drm/v3d/Makefile
+++ b/drivers/gpu/drm/v3d/Makefile
@@ -12,7 +12,8 @@ v3d-y := \
v3d_perfmon.o \
v3d_trace_points.o \
v3d_sched.o \
- v3d_sysfs.o
+ v3d_sysfs.o \
+ v3d_submit.o
v3d-$(CONFIG_DEBUG_FS) += v3d_debugfs.o
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index fc04372d5cbd..4db9ace66024 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -401,17 +401,19 @@ struct dma_fence *v3d_fence_create(struct v3d_dev *v3d,
enum v3d_queue queue);
/* v3d_gem.c */
int v3d_gem_init(struct drm_device *dev);
void v3d_gem_destroy(struct drm_device *dev);
+void v3d_reset(struct v3d_dev *v3d);
+void v3d_invalidate_caches(struct v3d_dev *v3d);
+void v3d_clean_caches(struct v3d_dev *v3d);
+
+/* v3d_submit.c */
+void v3d_job_cleanup(struct v3d_job *job);
+void v3d_job_put(struct v3d_job *job);
int v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
-void v3d_job_cleanup(struct v3d_job *job);
-void v3d_job_put(struct v3d_job *job);
-void v3d_reset(struct v3d_dev *v3d);
-void v3d_invalidate_caches(struct v3d_dev *v3d);
-void v3d_clean_caches(struct v3d_dev *v3d);
/* v3d_irq.c */
int v3d_irq_init(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 26f62a48d226..afc565078c78 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -11,8 +11,6 @@
#include
#include
-#include
-#include
#include "v3d_drv.h"
#include "v3d_regs.h"
@@ -241,739 +239,6 @@ v3d_invalidate_caches(struct v3d_dev *v3d)
v3d_invalidate_slices(v3d, 0);
}
-/* Takes the reservation lock on all the BOs being referenced, so that
- * at queue submit time we can update the reservations.
- *
- * We don't lock the RCL the tile alloc/state BOs, or overflow memory
- * (all of which are on exec->unref_list). They're entirely private
- * to v3d, so we don't attach dma-buf fences to them.
- */
-static int
-v3d_lock_bo_reservations(struct v3d_job *job,
-struct ww_acquire_ctx *acquire_ctx)
-{
- int i, ret;
-
- ret = drm_gem_lock_reservations(job->bo, job->bo_count, acquire_ctx);
- if (ret)
- return ret;
-
- for (i = 0; i < job->bo_count; i++) {
- ret = dma_resv_reserve_fences(job->bo[i]->resv, 1);
- if (ret)
- goto fail;
-
- ret = drm_sched_job_add_implicit_dependencies(>base,
- job->bo[i], true);
- if (ret)
- goto fail;
- }
-
- return 0;
-
-fail:
- drm_gem_unlock_reservations(job->bo, job->bo_count, acquire_ctx);
- return ret;
-}
-
-/**
- * v3d_lookup_bos() - Sets up job->bo[] with the GEM objects
- * referenced by the job.
- * @dev: DRM device
- * @file_priv: DRM file for this fd
- * @job: V3D job being set up
- * @bo_handles: GEM handles
- * @bo_count: Number of GEM handles passed in
- *
- * The command validator needs to reference BOs by their index within
- * the submitted job's BO list. This does the validation of the job's
- * BO list and reference counting for the lifetime of the job.
- *
- * Note that this function doesn't need to unreference the BOs on
- * failure, because that will happen at v3d_exec_cleanup() time.
- */
-static int
-v3d_lookup_bos(struct drm_device *dev,
- struct drm_file *file_priv,
- struct v3d_job *job,
- u64 bo_handles,
- u32 bo_count)
-{
- job->bo_count = bo_count;
-
- if (!job->bo_count) {
- /* See comment on bo_index for why we have to check
-* this.
-*/
- DRM_DEBUG("Rendering requires BOs\n");
- return -EINVAL;
- }
-
- return
[PATCH v3 04/17] drm/v3d: Simplify job refcount handling
From: Melissa Wen
Instead of checking if the job is NULL every time we call the function,
check it inside the function.
Signed-off-by: Melissa Wen
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_submit.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c
index f36214002f37..a0caf9c499bb 100644
--- a/drivers/gpu/drm/v3d/v3d_submit.c
+++ b/drivers/gpu/drm/v3d/v3d_submit.c
@@ -129,6 +129,9 @@ void v3d_job_cleanup(struct v3d_job *job)
void v3d_job_put(struct v3d_job *job)
{
+ if (!job)
+ return;
+
kref_put(>refcount, job->free);
}
@@ -517,11 +520,9 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
,
last_job->done_fence);
- if (bin)
- v3d_job_put(>base);
+ v3d_job_put(>base);
v3d_job_put(>base);
- if (clean_job)
- v3d_job_put(clean_job);
+ v3d_job_put(clean_job);
return 0;
--
2.42.0
[PATCH v3 02/17] drm/v3d: Move wait BO ioctl to the v3d_bo file
From: Melissa Wen
IOCTLs related to BO operations reside on the file v3d_bo.c. The wait BO
ioctl is the only IOCTL regarding BOs that is placed in a different file.
So, move it to the v3d_bo.c file.
Signed-off-by: Melissa Wen
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_bo.c | 33 +
drivers/gpu/drm/v3d/v3d_drv.h | 4 ++--
drivers/gpu/drm/v3d/v3d_gem.c | 33 -
3 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 8b3229a37c6d..357a0da7e16a 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -233,3 +233,36 @@ int v3d_get_bo_offset_ioctl(struct drm_device *dev, void
*data,
drm_gem_object_put(gem_obj);
return 0;
}
+
+int
+v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ int ret;
+ struct drm_v3d_wait_bo *args = data;
+ ktime_t start = ktime_get();
+ u64 delta_ns;
+ unsigned long timeout_jiffies =
+ nsecs_to_jiffies_timeout(args->timeout_ns);
+
+ if (args->pad != 0)
+ return -EINVAL;
+
+ ret = drm_gem_dma_resv_wait(file_priv, args->handle,
+ true, timeout_jiffies);
+
+ /* Decrement the user's timeout, in case we got interrupted
+* such that the ioctl will be restarted.
+*/
+ delta_ns = ktime_to_ns(ktime_sub(ktime_get(), start));
+ if (delta_ns < args->timeout_ns)
+ args->timeout_ns -= delta_ns;
+ else
+ args->timeout_ns = 0;
+
+ /* Asked to wait beyond the jiffie/scheduler precision? */
+ if (ret == -ETIME && args->timeout_ns)
+ ret = -EAGAIN;
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index d8b392494eba..fc04372d5cbd 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -385,6 +385,8 @@ int v3d_mmap_bo_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int v3d_get_bo_offset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+int v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
struct drm_gem_object *v3d_prime_import_sg_table(struct drm_device *dev,
struct dma_buf_attachment
*attach,
struct sg_table *sgt);
@@ -405,8 +407,6 @@ int v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int v3d_submit_csd_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
-int v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file_priv);
void v3d_job_cleanup(struct v3d_job *job);
void v3d_job_put(struct v3d_job *job);
void v3d_reset(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 9d2ac23c29e3..26f62a48d226 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -363,39 +363,6 @@ void v3d_job_put(struct v3d_job *job)
kref_put(>refcount, job->free);
}
-int
-v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
- struct drm_file *file_priv)
-{
- int ret;
- struct drm_v3d_wait_bo *args = data;
- ktime_t start = ktime_get();
- u64 delta_ns;
- unsigned long timeout_jiffies =
- nsecs_to_jiffies_timeout(args->timeout_ns);
-
- if (args->pad != 0)
- return -EINVAL;
-
- ret = drm_gem_dma_resv_wait(file_priv, args->handle,
- true, timeout_jiffies);
-
- /* Decrement the user's timeout, in case we got interrupted
-* such that the ioctl will be restarted.
-*/
- delta_ns = ktime_to_ns(ktime_sub(ktime_get(), start));
- if (delta_ns < args->timeout_ns)
- args->timeout_ns -= delta_ns;
- else
- args->timeout_ns = 0;
-
- /* Asked to wait beyond the jiffie/scheduler precision? */
- if (ret == -ETIME && args->timeout_ns)
- ret = -EAGAIN;
-
- return ret;
-}
-
static int
v3d_job_init(struct v3d_dev *v3d, struct drm_file *file_priv,
void **container, size_t size, void (*free)(struct kref *ref),
--
2.42.0
[PATCH v3 01/17] drm/v3d: Remove unused function header
From: Melissa Wen v3d_mmu_get_offset header was added but the function was never defined. Just remove it. Signed-off-by: Melissa Wen Signed-off-by: Maíra Canal --- drivers/gpu/drm/v3d/v3d_drv.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index 4c59fefaa0b4..d8b392494eba 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -420,8 +420,6 @@ void v3d_irq_disable(struct v3d_dev *v3d); void v3d_irq_reset(struct v3d_dev *v3d); /* v3d_mmu.c */ -int v3d_mmu_get_offset(struct drm_file *file_priv, struct v3d_bo *bo, - u32 *offset); int v3d_mmu_set_page_table(struct v3d_dev *v3d); void v3d_mmu_insert_ptes(struct v3d_bo *bo); void v3d_mmu_remove_ptes(struct v3d_bo *bo); -- 2.42.0
Re: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT
Am 27.11.23 um 17:47 schrieb Bhardwaj, Rajneesh: [AMD Official Use Only - General] -Original Message- From: amd-gfx On Behalf Of Hamza Mahfooz Sent: Monday, November 27, 2023 10:53 AM To: Christian König ; jani.nik...@linux.intel.com; kher...@redhat.com; d...@redhat.com; za...@vmware.com; Olsak, Marek ; linux-graphics-maintai...@vmware.com; amd-...@lists.freedesktop.org; nouv...@lists.freedesktop.org; intel-...@lists.freedesktop.org; virtualizat...@lists.linux.dev; spice-de...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT On 11/27/23 09:54, Christian König wrote: Try to fill up VRAM as well by setting the busy flag on GTT allocations. This fixes the issue that when VRAM was evacuated for suspend it's never filled up again unless the application is restarted. I found the subject description a bit misleading. Maybe use a Fixes tag describing it is a fix for suspend resume regression other than that, looks good to me. Well exactly that's the problem, this isn't really a fix and we also don't want to backport it. Basically the previous behavior was working as design, it's just that it was never intended to be used like this. Acked-by: Rajneesh Bhardwaj Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2893 Thanks, Christian. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index aa0dd6dad068..ddc8fb4db678 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain) abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? AMDGPU_PL_PREEMPT : TTM_PL_TT; places[c].flags = 0; + /* + * When GTT is just an alternative to VRAM make sure that we + * only use it as fallback and still try to fill up VRAM first. + */ + if (domain & AMDGPU_GEM_DOMAIN_VRAM) + places[c].flags |= TTM_PL_FLAG_BUSY; c++; } -- Hamza
Re: [PATCH] Replace ioremap_cache() with memremap()
Wei, this is one of the Hyper-V code improvement tasks that Michael Kelley identified. Using memremap() is the right thing to do here. Abhinav Singh (cc'ed) also tried to fix this earlier as there are sparse warnings with ioremap_cache(). On Sun, Nov 12, 2023 at 11:12:33PM +, Wei Liu wrote: > On Wed, Nov 01, 2023 at 09:01:48AM -0700, Nischala Yelchuri wrote: > > Current Hyper-V code for CoCo VMs uses ioremap_cache() to map normal memory > > as decrypted. > > ioremap_cache() is ideally used to map I/O device memory when it has the > > characteristics > > of normal memory. It should not be used to map normal memory as the > > returned pointer > > has the __iomem attribute. > > Do you find any real world issues with the current code? How do you > discover these issues? > > Thanks, > Wei.
Re: [PATCH 04/12] dt-bindings: display: msm: sm8450-mdss: Allow 3 interconnects
On Sat, 25 Nov 2023 15:17:32 +0100, Konrad Dybcio wrote: > In addition to MDP01, the cpu-cfg interconnect is also necessary. > Allow it. > > Signed-off-by: Konrad Dybcio > --- > Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.example.dtb: display-subsystem@ae0: interconnects: [[4294967295, 14, 0, 4294967295, 3, 0], [4294967295, 14, 0, 4294967295, 3, 0]] is too short from schema $id: http://devicetree.org/schemas/display/msm/qcom,sm8450-mdss.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.example.dtb: display-subsystem@ae0: interconnect-names: ['mdp0-mem', 'mdp1-mem'] is too short from schema $id: http://devicetree.org/schemas/display/msm/qcom,sm8450-mdss.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231125-topic-rb1_feat-v1-4-11d71b12b...@linaro.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH 03/12] dt-bindings: display: msm: qcm2290-mdss: Allow 2 interconnects
On Sat, 25 Nov 2023 15:17:31 +0100, Konrad Dybcio wrote: > In addition to MDP0, the cpu-cfg interconnect is also necessary. > Allow it. > > Signed-off-by: Konrad Dybcio > --- > Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.example.dtb: display-subsystem@5e0: interconnects: [[4294967295, 1, 4294967295, 6]] is too short from schema $id: http://devicetree.org/schemas/display/msm/qcom,qcm2290-mdss.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.example.dtb: display-subsystem@5e0: interconnect-names: ['mdp0-mem'] is too short from schema $id: http://devicetree.org/schemas/display/msm/qcom,qcm2290-mdss.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231125-topic-rb1_feat-v1-3-11d71b12b...@linaro.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules
On Mon, Nov 27, 2023 at 05:38:13PM +0100, Maxime Ripard wrote: > Greg, Rafael, > > On Mon, Nov 27, 2023 at 01:14:13PM +0800, Liu Ying wrote: > > Export device_is_dependent() since the drm_kms_helper module is starting > > to use it. > > > > Signed-off-by: Liu Ying > > --- > > v2: > > * Newly introduced as needed by patch 2. > > > > drivers/base/core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index 67ba592afc77..bfd2bf0364b7 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void > > *target) > > } > > return ret; > > } > > +EXPORT_SYMBOL_GPL(device_is_dependent); > > So, a committer just applied this to drm-misc-fixes without your > approval. Could you ack it? If you don't want to, we'll fix it. Wait, why exactly is this needed? Nothing outside of the driver core should be needing this function, it shouldn't be public at all (I missed that before.) So please, revert it for now, let's figure out why DRM thinks this is needed for it's devices, and yet no other bus/subsystem does. thanks, greg k-h
Re: [PATCH] dt-bindings: display: mediatek: dsi: remove Xinlei's mail
On Thu, 23 Nov 2023 14:49:27 +0100, Michael Walle wrote: > Xinlei Lee's mail is bouncing: > > : host mailgw02.mediatek.com[216.200.240.185] said: > 550 Relaying mail to xinlei@mediatek.com is not allowed (in reply to > RCPT TO command) > > Remove it. > > Signed-off-by: Michael Walle > --- > .../devicetree/bindings/display/mediatek/mediatek,dsi.yaml | 1 - > 1 file changed, 1 deletion(-) > Applied, thanks!
Re: [PATCH drm-misc-next 4/5] drm/gpuvm: fall back to drm_exec_lock_obj()
Hi,
On 11/27/23 14:52, Christian König wrote:
Am 25.11.23 um 00:40 schrieb Danilo Krummrich:
Hi Christian,
do you think it makes sense to handle this in drm_exec_prepare_obj() or
even dma_resv_reserve_fences() even?
IIRC for drm_exec the discussion has gone a bit back and forth between handling
0 and having a separate function which doesn't allocate fences.
We ended up with the solution using separate calls since you either know that
you don't need fences (because you for example only need to look something up)
or you need fences and eventually calculate the number you need dynamically and
if you then end up with 0 it's a bug.
I don't have a strong opinion on that. Though, in GPUVM I'd probably still need
some wrapper like
+exec_prepare_obj(struct drm_exec *exec, struct drm_gem_object *obj,
+ unsigned int num_fences)
+{
+return num_fences ? drm_exec_prepare_obj(exec, obj, num_fences) :
+drm_exec_lock_obj(exec, obj);
+}
to prevent either duplicate code or rather unnecessary complicated abstractions.
And I'm not sure it's super nice that drm_gpuvm_prepare_objects() allows zero
fences, whereas drm_exec_prepare_obj() does not.
So to sum it up the conclusion was that it's more defensive to complain about 0
fences to reserve (which reminds me that dma_resv_reserve_fences() still needs
to get a warning for 0 fences as well).
What's the logic you'd want to apply there? WARN() but still allocate at least
4 slots or WARN() and return doing nothing?
- Danilo
Regards,
Christian.
- Danilo
On 11/25/23 00:36, Danilo Krummrich wrote:
Fall back to drm_exec_lock_obj() if num_fences is zero for the
drm_gpuvm_prepare_* function family.
Otherwise dma_resv_reserve_fences() would actually allocate slots even
though num_fences is zero.
Cc: Christian König
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/drm_gpuvm.c | 36 +---
include/drm/drm_gpuvm.h | 23 +++
2 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index 54f5e8851de5..d1d1c2379e44 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -1085,6 +1085,36 @@ drm_gpuvm_put(struct drm_gpuvm *gpuvm)
}
EXPORT_SYMBOL_GPL(drm_gpuvm_put);
+static int
+exec_prepare_obj(struct drm_exec *exec, struct drm_gem_object *obj,
+ unsigned int num_fences)
+{
+ return num_fences ? drm_exec_prepare_obj(exec, obj, num_fences) :
+ drm_exec_lock_obj(exec, obj);
+}
+
+/**
+ * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
+ * @gpuvm: the _gpuvm
+ * @exec: the _exec context
+ * @num_fences: the amount of _fences to reserve
+ *
+ * Calls drm_exec_prepare_obj() for the GPUVMs dummy _gem_object.
+ *
+ * Using this function directly, it is the drivers responsibility to call
+ * drm_exec_init() and drm_exec_fini() accordingly.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int
+drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
+ struct drm_exec *exec,
+ unsigned int num_fences)
+{
+ return exec_prepare_obj(exec, gpuvm->r_obj, num_fences);
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_vm);
+
static int
__drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
struct drm_exec *exec,
@@ -1095,7 +1125,7 @@ __drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
int ret = 0;
for_each_vm_bo_in_list(gpuvm, extobj, , vm_bo) {
- ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
+ ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
if (ret)
break;
}
@@ -1116,7 +1146,7 @@ drm_gpuvm_prepare_objects_locked(struct drm_gpuvm *gpuvm,
drm_gpuvm_resv_assert_held(gpuvm);
list_for_each_entry(vm_bo, >extobj.list, list.entry.extobj) {
- ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences);
+ ret = exec_prepare_obj(exec, vm_bo->obj, num_fences);
if (ret)
break;
@@ -1186,7 +1216,7 @@ drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, struct
drm_exec *exec,
drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) {
struct drm_gem_object *obj = va->gem.obj;
- ret = drm_exec_prepare_obj(exec, obj, num_fences);
+ ret = exec_prepare_obj(exec, obj, num_fences);
if (ret)
return ret;
}
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index f94fec9a8517..b3f82ec7fb17 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -544,26 +544,9 @@ struct drm_gpuvm_exec {
} extra;
};
-/**
- * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
- * @gpuvm: the _gpuvm
- * @exec: the _exec context
- * @num_fences: the amount of _fences to reserve
- *
- * Calls drm_exec_prepare_obj() for the GPUVMs dummy _gem_object.
- *
- * Using this function directly, it is the drivers responsibility to call
- *
Re: [PATCH] dt-bindings: mxsfb: Remove power-domains requirements for i.MX7D
On Mon, Nov 27, 2023 at 04:35:56PM +0100, Roland Hieber wrote:
> From: Philipp Zabel
>
> i.MX7D is documented as compatible with i.MX6SX, but it doesn't have the
> power-domain requirement. Limit the i.MX6SX power-domains requirement to
> i.MX6SX only.
I dont like this commit message tbh, I shouldn't have to go and dig out the
file to be able to understand what you are doing here - it should really be
explained that the imx7d falls back to the imx6sx, as otherwise this looks
like nothing at all to do with with imx7d.
Thanks,
Conor.
>
> Fixes: f62678a77d58 ("dt-bindings: mxsfb: Document i.MX8M/i.MX6SX/i.MX6SL
> power-domains property")
> Signed-off-by: Philipp Zabel
> Signed-off-by: Roland Hieber
> ---
> Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> index fc11ab5fc465..83532959e51c 100644
> --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml
> @@ -117,13 +117,19 @@ allOf:
>maxItems: 1
> clock-names:
>maxItems: 1
> + - if:
> + properties:
> +compatible:
> + const: fsl,imx6sx-lcdif
> +then:
> + required:
> +- power-domains
>- if:
>properties:
> compatible:
>contains:
> enum:
>- fsl,imx6sl-lcdif
> - - fsl,imx6sx-lcdif
>- fsl,imx8mm-lcdif
>- fsl,imx8mn-lcdif
>- fsl,imx8mp-lcdif
>
> ---
> base-commit: 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab
> change-id: 20231127-b4-dt-bindings-mxsfb-9a0676d08b4c
>
> Best regards,
> --
> Roland Hieber, Pengutronix e.K. | r...@pengutronix.de |
> Steuerwalder Str. 21 | https://www.pengutronix.de/ |
> 31137 Hildesheim, Germany| Phone: +49-5121-206917-0|
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
>
>
signature.asc
Description: PGP signature
Re: [PATCH 1/2] dt-bindings: display: bridge: samsung-dsim: Add i.MX7D support
On Mon, Nov 27, 2023 at 05:12:28PM +0100, Roland Hieber wrote: > From: Philipp Zabel > > Add support for the "fsl,imx7d-mipi-dsim" compatible used on i.MX7D. > > Signed-off-by: Philipp Zabel > Signed-off-by: Roland Hieber Acked-by: Conor Dooley Cheers, Conor. > --- > .../devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml | 4 > +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > index 4ed7a799ba26..e43fec560941 100644 > --- a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml > @@ -27,7 +27,9 @@ properties: >- fsl,imx8mm-mipi-dsim >- fsl,imx8mp-mipi-dsim >- items: > - - const: fsl,imx8mn-mipi-dsim > + - enum: > + - fsl,imx7d-mipi-dsim > + - fsl,imx8mn-mipi-dsim >- const: fsl,imx8mm-mipi-dsim > >reg: > > -- > 2.39.2 > signature.asc Description: PGP signature
Re: [Linaro-mm-sig] Re: [PATCH] dma-buf: fix check in dma_resv_add_fence
On 11/27/23 14:24, Christian König wrote:
Ping? Can I get an rb or acked-by for that?
Thanks,
Christian.
Am 15.11.23 um 10:30 schrieb Christian König:
It's valid to add the same fence multiple times to a dma-resv object and
we shouldn't need one extra slot for each.
Signed-off-by: Christian König
Fixes: a3f7c10a269d5 ("dma-buf/dma-resv: check if the new fence is
really later")
Cc: sta...@vger.kernel.org # v5.19+
Reviewed-by: Thomas Hellström
---
drivers/dma-buf/dma-resv.c | 2 +-
include/linux/dma-fence.h | 15 +++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 38b4110378de..eb8b733065b2 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -301,7 +301,7 @@ void dma_resv_add_fence(struct dma_resv *obj,
struct dma_fence *fence,
dma_resv_list_entry(fobj, i, obj, , _usage);
if ((old->context == fence->context && old_usage >= usage &&
- dma_fence_is_later(fence, old)) ||
+ dma_fence_is_later_or_same(fence, old)) ||
dma_fence_is_signaled(old)) {
dma_resv_list_set(fobj, i, fence, usage);
dma_fence_put(old);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index ebe78bd3d121..b3772edca2e6 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -498,6 +498,21 @@ static inline bool dma_fence_is_later(struct
dma_fence *f1,
return __dma_fence_is_later(f1->seqno, f2->seqno, f1->ops);
}
+/**
+ * dma_fence_is_later_or_same - return true if f1 is later or same
as f2
+ * @f1: the first fence from the same context
+ * @f2: the second fence from the same context
+ *
+ * Returns true if f1 is chronologically later than f2 or the same
fence. Both
+ * fences must be from the same context, since a seqno is not
re-used across
+ * contexts.
+ */
+static inline bool dma_fence_is_later_or_same(struct dma_fence *f1,
+ struct dma_fence *f2)
+{
+ return f1 == f2 || dma_fence_is_later(f1, f2);
+}
+
/**
* dma_fence_later - return the chronologically later fence
* @f1: the first fence from the same context
___
Linaro-mm-sig mailing list -- linaro-mm-...@lists.linaro.org
To unsubscribe send an email to linaro-mm-sig-le...@lists.linaro.org
RE: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT
[AMD Official Use Only - General] -Original Message- From: amd-gfx On Behalf Of Hamza Mahfooz Sent: Monday, November 27, 2023 10:53 AM To: Christian König ; jani.nik...@linux.intel.com; kher...@redhat.com; d...@redhat.com; za...@vmware.com; Olsak, Marek ; linux-graphics-maintai...@vmware.com; amd-...@lists.freedesktop.org; nouv...@lists.freedesktop.org; intel-...@lists.freedesktop.org; virtualizat...@lists.linux.dev; spice-de...@lists.freedesktop.org; dri-devel@lists.freedesktop.org Subject: Re: [PATCH 2/2] drm/amdgpu: use GTT only as fallback for VRAM|GTT On 11/27/23 09:54, Christian König wrote: > Try to fill up VRAM as well by setting the busy flag on GTT allocations. > > This fixes the issue that when VRAM was evacuated for suspend it's > never filled up again unless the application is restarted. I found the subject description a bit misleading. Maybe use a Fixes tag describing it is a fix for suspend resume regression other than that, looks good to me. Acked-by: Rajneesh Bhardwaj > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2893 > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index aa0dd6dad068..ddc8fb4db678 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -173,6 +173,12 @@ void amdgpu_bo_placement_from_domain(struct amdgpu_bo > *abo, u32 domain) > abo->flags & AMDGPU_GEM_CREATE_PREEMPTIBLE ? > AMDGPU_PL_PREEMPT : TTM_PL_TT; > places[c].flags = 0; > + /* > + * When GTT is just an alternative to VRAM make sure that we > + * only use it as fallback and still try to fill up VRAM first. > + */ > + if (domain & AMDGPU_GEM_DOMAIN_VRAM) > + places[c].flags |= TTM_PL_FLAG_BUSY; > c++; > } > -- Hamza
Re: [PATCH v9 07/12] clk: meson: add vclk driver
On Mon 27 Nov 2023 at 17:14, Neil Armstrong wrote:
> On 24/11/2023 15:41, Jerome Brunet wrote:
>> On Fri 24 Nov 2023 at 09:41, Neil Armstrong
>> wrote:
>>
>>> The VCLK and VCLK_DIV clocks have supplementary bits.
>>>
>>> The VCLK has a "SOFT RESET" bit to toggle after the whole
>>> VCLK sub-tree rate has been set, this is implemented in
>>> the gate enable callback.
>>>
>>> The VCLK_DIV clocks as enable and reset bits used to disable
>>> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
>>> the rate is set while the divider is disabled and in reset mode.
>>>
>>> The VCLK_DIV enable bit isn't implemented as a gate since it's part
>>> of the divider logic and vendor does this exact sequence to ensure
>>> the divider is correctly set.
>>>
>>> Signed-off-by: Neil Armstrong
>>> ---
>>> drivers/clk/meson/Kconfig | 5 ++
>>> drivers/clk/meson/Makefile | 1 +
>>> drivers/clk/meson/vclk.c | 141
>>> +
>>> drivers/clk/meson/vclk.h | 51
>>> 4 files changed, 198 insertions(+)
>>>
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index 29ffd14d267b..59a40a49f8e1 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>> tristate
>>> select COMMON_CLK_MESON_REGMAP
>>> +config COMMON_CLK_MESON_VCLK
>>> + tristate
>>> + select COMMON_CLK_MESON_REGMAP
>>> +
>>> config COMMON_CLK_MESON_CLKC_UTILS
>>> tristate
>>> @@ -140,6 +144,7 @@ config COMMON_CLK_G12A
>>> select COMMON_CLK_MESON_EE_CLKC
>>> select COMMON_CLK_MESON_CPU_DYNDIV
>>> select COMMON_CLK_MESON_VID_PLL_DIV
>>> + select COMMON_CLK_MESON_VCLK
>> This particular line belong in the next patch
>>
>>> select MFD_SYSCON
>>> help
>>> Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index 9ee4b954c896..9ba43fe7a07a 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>>> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>>> # Amlogic Clock controllers
>>> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
>>> new file mode 100644
>>> index ..47f08a52b49f
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/vclk.c
>>> @@ -0,0 +1,141 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2023 Neil Armstrong
>>> + */
>>> +
>>> +#include
>>> +#include "vclk.h"
>>> +
>>> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
>>> +
>>> +static inline struct clk_regmap_vclk_data *
>>> +clk_get_regmap_vclk_data(struct clk_regmap *clk)
>>> +{
>>> + return (struct clk_regmap_vclk_data *)clk->data;
>>> +}
>>> +
>>> +static int clk_regmap_vclk_enable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
>>> +
>>> + meson_parm_write(clk->map, >enable, 1);
>>> +
>>> + /* Do a reset pulse */
>>> + meson_parm_write(clk->map, >reset, 1);
>>> + meson_parm_write(clk->map, >reset, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void clk_regmap_vclk_disable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
>>> +
>>> + meson_parm_write(clk->map, >enable, 0);
>>> +}
>>> +
>>> +static int clk_regmap_vclk_is_enabled(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
>>> +
>>> + return meson_parm_read(clk->map, >enable);
>>> +}
>>> +
>>> +const struct clk_ops clk_regmap_vclk_ops = {
>>> + .enable = clk_regmap_vclk_enable,
>>> + .disable = clk_regmap_vclk_disable,
>>> + .is_enabled = clk_regmap_vclk_is_enabled,
>>> +};
>>> +EXPORT_SYMBOL_GPL(clk_regmap_vclk_ops);
>> s/clk_regmap_vclk/meson_vclk at least for what is exported, ideally most
>> all the code.
>> I get clk_regmap_ comes from code copied from clk_regmap.c.
>> The reason the this part is different (and not using parm) if that when
>> I converted amlogic to regmap, I hope we could make this generic,
>> possibly converging between aml and qcom (which was the only other
>> platform using regmap for clock at the time). This is why clk_regmap.c
>> is a bit different from the other driver.
>> For the aml specific drivers, best to look at the mpll or cpu-dyndiv one.
>>
>>> +
>>> +/* The VCLK Divider has supplementary reset & enable bits */
>>> +
>>> +static inline struct clk_regmap_vclk_div_data *
>>>
Re: [PATCH v2 1/2] driver core: Export device_is_dependent() to modules
Greg, Rafael, On Mon, Nov 27, 2023 at 01:14:13PM +0800, Liu Ying wrote: > Export device_is_dependent() since the drm_kms_helper module is starting > to use it. > > Signed-off-by: Liu Ying > --- > v2: > * Newly introduced as needed by patch 2. > > drivers/base/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 67ba592afc77..bfd2bf0364b7 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -328,6 +328,7 @@ int device_is_dependent(struct device *dev, void *target) > } > return ret; > } > +EXPORT_SYMBOL_GPL(device_is_dependent); So, a committer just applied this to drm-misc-fixes without your approval. Could you ack it? If you don't want to, we'll fix it. Maxime signature.asc Description: PGP signature
Re: [PATCH v2 0/2] drm/bridge: panel: Check device dependency before managing device link
On Mon, Nov 27, 2023 at 05:03:53PM +0100, Linus Walleij wrote: > On Mon, Nov 27, 2023 at 6:10 AM Liu Ying wrote: > > > This series aims to check panel device dependency upon DRM device before > > managing device link between them. It fixes eariler patches in v6.7-rc1 > > which tried to manage the link. Without this series, the link fails to > > be added for dependent panel devices and hence relevant panel bridges > > fail to be attached. A real broken panel is "novatek,nt35510" defined > > in arch/arm/boot/dts/st/ste-ux500-samsung-skomer.dts as reported by > > Linus Walleij. > > > > Patch 1 exports device_is_dependent() to modules as needed by patch 2. > > Patch 2 checks device dependency before managing the device link. > > > > Note that patch 2 is already in drm-misc/drm-misc-fixes and > > drm-misc/for-linux-next-fixes. Patch 1 needs to be reviewed and picked up. > > > > v2: > > * Introduce patch 1 to export device_is_dependent() to modules as needed by > > patch 2. > > > > Liu Ying (2): > > driver core: Export device_is_dependent() to modules > > drm/bridge: panel: Check device dependency before managing device link > > I just applied patch 1 directly to the drm-misc-fixes so we don't have to > revert and then re-apply patches, because that is a bigger evil. (We can't > rebase these branches...) Erm, you did wait for GKH or Rafael's ACK to do that, right? Maxime signature.asc Description: PGP signature
Re: [PATCH v2] drm/atomic-helpers: Invoke end_fb_access while owning plane state
Thomas Zimmermann writes:
> Invoke drm_plane_helper_funcs.end_fb_access before
> drm_atomic_helper_commit_hw_done(). The latter function hands over
> ownership of the plane state to the following commit, which might
> free it. Releasing resources in end_fb_access then operates on undefined
> state. This bug has been observed with non-blocking commits when they
> are being queued up quickly.
>
> Here is an example stack trace from the bug report. The plane state has
> been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
>
> Unable to handle kernel paging request at virtual address 00010049
> [...]
> drm_gem_fb_vunmap+0x18/0x74
> drm_gem_end_shadow_fb_access+0x1c/0x2c
> drm_atomic_helper_cleanup_planes+0x58/0xd8
> drm_atomic_helper_commit_tail+0x90/0xa0
> commit_tail+0x15c/0x188
> commit_work+0x14/0x20
>
> For aborted commits, it is still ok to run end_fb_access as part of the
> plane's cleanup. Add a test to drm_atomic_helper_cleanup_planes().
>
> v2:
> * fix test in drm_atomic_helper_cleanup_planes()
>
> Reported-by: Alyssa Ross
> Closes: https://lore.kernel.org/dri-devel/87leazm0ya@alyssa.is/
> Suggested-by: Daniel Vetter
> Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane
> helpers")
> Signed-off-by: Thomas Zimmermann
> Cc: # v6.2+
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 17 +
> 1 file changed, 17 insertions(+)
Got this basically immediately. :(
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_init] Allocated
atomic state cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_plane_state]
Added [PLANE:31:plane-0] 4935bdca state to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_crtc_state] Added
[CRTC:33:crtc-0] d25f613d state to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_set_fb_for_plane] Set
[FB:38] for [PLANE:31:plane-0] state 4935bdca
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_connector_state]
Added [CONNECTOR:35:Unknown-1] 20d19f10 state to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_check_only] checking
cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
Updating routing for [CONNECTOR:35:Unknown-1]
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
[CONNECTOR:35:Unknown-1] keeps [ENCODER:34:None-34], now on [CRTC:33:crtc-0]
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_nonblocking_commit]
committing cfb3f1f2 nonblocking
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_default_clear]
Clearing atomic state cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:__drm_atomic_state_free] Freeing
atomic state cfb3f1f2
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_init] Allocated
atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_plane_state]
Added [PLANE:31:plane-0] 83f22dc6 state to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_crtc_state] Added
[CRTC:33:crtc-0] eec339c5 state to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_set_fb_for_plane] Set
[FB:37] for [PLANE:31:plane-0] state 83f22dc6
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_connector_state]
Added [CONNECTOR:35:Unknown-1] 22495ce9 state to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_check_only] checking
03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
Updating routing for [CONNECTOR:35:Unknown-1]
simple-framebuffer dd53a4000.framebuffer: [drm:update_connector_routing]
[CONNECTOR:35:Unknown-1] keeps [ENCODER:34:None-34], now on [CRTC:33:crtc-0]
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_add_encoder_bridges]
Adding all bridges for [encoder:34:None-34] to 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_default_clear]
Clearing atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:__drm_atomic_state_free] Freeing
atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_state_init] Allocated
atomic state 03dc0c0b
simple-framebuffer dd53a4000.framebuffer: [drm:drm_atomic_get_plane_state]
Added [PLANE:31:plane-0] 83f22dc6 state to 03dc0c0b
Re: [PATCH v2 2/4] eventfd: simplify eventfd_signal()
On Wed, 2023-11-22 at 13:48 +0100, Christian Brauner wrote:
> Ever since the evenfd type was introduced back in 2007 in commit
s/evenfd/eventfd/
> e1ad7468c77d ("signal/timer/event: eventfd core") the
> eventfd_signal()
> function only ever passed 1 as a value for @n. There's no point in
> keeping that additional argument.
>
> Signed-off-by: Christian Brauner
> ---
> arch/x86/kvm/hyperv.c | 2 +-
> arch/x86/kvm/xen.c | 2 +-
> drivers/accel/habanalabs/common/device.c | 2 +-
> drivers/fpga/dfl.c | 2 +-
> drivers/gpu/drm/drm_syncobj.c | 6 +++---
> drivers/gpu/drm/i915/gvt/interrupt.c | 2 +-
> drivers/infiniband/hw/mlx5/devx.c | 2 +-
> drivers/misc/ocxl/file.c | 2 +-
> drivers/s390/cio/vfio_ccw_chp.c | 2 +-
> drivers/s390/cio/vfio_ccw_drv.c | 4 ++--
> drivers/s390/cio/vfio_ccw_ops.c | 6 +++---
> drivers/s390/crypto/vfio_ap_ops.c | 2 +-
Acked-by: Eric Farman # s390
Re: [PATCH v9 07/12] clk: meson: add vclk driver
On 24/11/2023 15:41, Jerome Brunet wrote:
On Fri 24 Nov 2023 at 09:41, Neil Armstrong wrote:
The VCLK and VCLK_DIV clocks have supplementary bits.
The VCLK has a "SOFT RESET" bit to toggle after the whole
VCLK sub-tree rate has been set, this is implemented in
the gate enable callback.
The VCLK_DIV clocks as enable and reset bits used to disable
and reset the divider, associated with CLK_SET_RATE_GATE it ensures
the rate is set while the divider is disabled and in reset mode.
The VCLK_DIV enable bit isn't implemented as a gate since it's part
of the divider logic and vendor does this exact sequence to ensure
the divider is correctly set.
Signed-off-by: Neil Armstrong
---
drivers/clk/meson/Kconfig | 5 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/vclk.c | 141 +
drivers/clk/meson/vclk.h | 51
4 files changed, 198 insertions(+)
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 29ffd14d267b..59a40a49f8e1 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
tristate
select COMMON_CLK_MESON_REGMAP
+config COMMON_CLK_MESON_VCLK
+ tristate
+ select COMMON_CLK_MESON_REGMAP
+
config COMMON_CLK_MESON_CLKC_UTILS
tristate
@@ -140,6 +144,7 @@ config COMMON_CLK_G12A
select COMMON_CLK_MESON_EE_CLKC
select COMMON_CLK_MESON_CPU_DYNDIV
select COMMON_CLK_MESON_VID_PLL_DIV
+ select COMMON_CLK_MESON_VCLK
This particular line belong in the next patch
select MFD_SYSCON
help
Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 9ee4b954c896..9ba43fe7a07a 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
+obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
# Amlogic Clock controllers
diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
new file mode 100644
index ..47f08a52b49f
--- /dev/null
+++ b/drivers/clk/meson/vclk.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Neil Armstrong
+ */
+
+#include
+#include "vclk.h"
+
+/* The VCLK gate has a supplementary reset bit to pulse after ungating */
+
+static inline struct clk_regmap_vclk_data *
+clk_get_regmap_vclk_data(struct clk_regmap *clk)
+{
+ return (struct clk_regmap_vclk_data *)clk->data;
+}
+
+static int clk_regmap_vclk_enable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
+
+ meson_parm_write(clk->map, >enable, 1);
+
+ /* Do a reset pulse */
+ meson_parm_write(clk->map, >reset, 1);
+ meson_parm_write(clk->map, >reset, 0);
+
+ return 0;
+}
+
+static void clk_regmap_vclk_disable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
+
+ meson_parm_write(clk->map, >enable, 0);
+}
+
+static int clk_regmap_vclk_is_enabled(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct clk_regmap_vclk_data *vclk = clk_get_regmap_vclk_data(clk);
+
+ return meson_parm_read(clk->map, >enable);
+}
+
+const struct clk_ops clk_regmap_vclk_ops = {
+ .enable = clk_regmap_vclk_enable,
+ .disable = clk_regmap_vclk_disable,
+ .is_enabled = clk_regmap_vclk_is_enabled,
+};
+EXPORT_SYMBOL_GPL(clk_regmap_vclk_ops);
s/clk_regmap_vclk/meson_vclk at least for what is exported, ideally most
all the code.
I get clk_regmap_ comes from code copied from clk_regmap.c.
The reason the this part is different (and not using parm) if that when
I converted amlogic to regmap, I hope we could make this generic,
possibly converging between aml and qcom (which was the only other
platform using regmap for clock at the time). This is why clk_regmap.c
is a bit different from the other driver.
For the aml specific drivers, best to look at the mpll or cpu-dyndiv one.
+
+/* The VCLK Divider has supplementary reset & enable bits */
+
+static inline struct clk_regmap_vclk_div_data *
+clk_get_regmap_vclk_div_data(struct clk_regmap *clk)
+{
+ return (struct clk_regmap_vclk_div_data *)clk->data;
+}
+
+static unsigned long clk_regmap_vclk_div_recalc_rate(struct clk_hw *hw,
+unsigned long prate)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct clk_regmap_vclk_div_data *vclk =
clk_get_regmap_vclk_div_data(clk);
+
+ return divider_recalc_rate(hw, prate,
[PATCH 2/2] ARM: dts: imx7: add MIPI-DSI support
From: Marco Felsch
This adds the device tree support for the MIPI-DSI block. The block can
be used as encoder for the parallel signals coming from the lcdif block.
Signed-off-by: Marco Felsch
Signed-off-by: Roland Hieber
---
arch/arm/boot/dts/nxp/imx/imx7s.dtsi | 46
1 file changed, 46 insertions(+)
diff --git a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
index 29b8fd03567a..7adadf9c3694 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
@@ -818,6 +818,16 @@ lcdif: lcdif@3073 {
< IMX7D_LCDIF_PIXEL_ROOT_CLK>;
clock-names = "pix", "axi";
status = "disabled";
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ lcdif_out_mipi_dsi: endpoint@0 {
+ reg = <0>;
+ remote-endpoint =
<_dsi_in_lcdif>;
+ };
+ };
};
mipi_csi: mipi-csi@3075 {
@@ -850,6 +860,42 @@ mipi_vc0_to_csi_mux: endpoint {
};
};
};
+
+ mipi_dsi: dsi@3076 {
+ compatible = "fsl,imx7d-mipi-dsim",
"fsl,imx8mm-mipi-dsim";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x3076 0x400>;
+ clocks = < IMX7D_MIPI_DSI_ROOT_CLK>,
+< IMX7D_MIPI_DPHY_ROOT_CLK>;
+ clock-names = "bus_clk", "sclk_mipi";
+ assigned-clocks = <
IMX7D_MIPI_DSI_ROOT_SRC>,
+ <
IMX7D_PLL_SYS_PFD5_CLK>;
+ assigned-clock-parents = <
IMX7D_PLL_SYS_PFD5_CLK>;
+ assigned-clock-rates = <0>, <33300>;
+ power-domains = <_mipi_phy>;
+ interrupts = ;
+ samsung,burst-clock-frequency = <89100>;
+ samsung,esc-clock-frequency = <2000>;
+ samsung,pll-clock-frequency = <2400>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mipi_dsi_in_lcdif: endpoint@0 {
+ reg = <0>;
+ remote-endpoint =
<_out_mipi_dsi>;
+ };
+ };
+ };
+ };
};
aips3: bus@3080 {
--
2.39.2
[PATCH 1/2] dt-bindings: display: bridge: samsung-dsim: Add i.MX7D support
From: Philipp Zabel Add support for the "fsl,imx7d-mipi-dsim" compatible used on i.MX7D. Signed-off-by: Philipp Zabel Signed-off-by: Roland Hieber --- .../devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml index 4ed7a799ba26..e43fec560941 100644 --- a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml +++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml @@ -27,7 +27,9 @@ properties: - fsl,imx8mm-mipi-dsim - fsl,imx8mp-mipi-dsim - items: - - const: fsl,imx8mn-mipi-dsim + - enum: + - fsl,imx7d-mipi-dsim + - fsl,imx8mn-mipi-dsim - const: fsl,imx8mm-mipi-dsim reg: -- 2.39.2
[PATCH 0/2] ARM: dts: imx7: add MIPI-DSI support
Signed-off-by: Roland Hieber --- Marco Felsch (1): ARM: dts: imx7: add MIPI-DSI support Philipp Zabel (1): dt-bindings: display: bridge: samsung-dsim: Add i.MX7D support .../bindings/display/bridge/samsung,mipi-dsim.yaml | 4 +- arch/arm/boot/dts/nxp/imx/imx7s.dtsi | 46 ++ 2 files changed, 49 insertions(+), 1 deletion(-) --- base-commit: 2cc14f52aeb78ce3f29677c2de1f06c0e91471ab change-id: 20231127-b4-imx7-mipi-dsi-801372e24f13 Best regards, -- Roland Hieber, Pengutronix e.K. | r...@pengutronix.de | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany| Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH] drm/sched: Partial revert of "Qualify drm_sched_wakeup() by drm_sched_entity_is_ready()"
Commit f3123c2590005c, in combination with the use of work queues by the GPU
scheduler, leads to random lock-ups of the GUI.
This is a partial revert of of commit f3123c2590005c since drm_sched_wakeup()
still
needs its entity argument to pass it to drm_sched_can_queue().
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2994
Link: https://lists.freedesktop.org/archives/dri-devel/2023-November/431606.html
Fixes: f3123c2590005c ("drm/sched: Qualify drm_sched_wakeup() by
drm_sched_entity_is_ready()")
Signed-off-by: Bert Karwatzki
---
drivers/gpu/drm/scheduler/sched_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 682aebe96db7..550492a7a031 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1029,9 +1029,8 @@ EXPORT_SYMBOL(drm_sched_job_cleanup);
void drm_sched_wakeup(struct drm_gpu_scheduler *sched,
struct drm_sched_entity *entity)
{
- if (drm_sched_entity_is_ready(entity))
- if (drm_sched_can_queue(sched, entity))
- drm_sched_run_job_queue(sched);
+ if (drm_sched_can_queue(sched, entity))
+ drm_sched_run_job_queue(sched);
}
/**
--
2.43.0
Re: [PATCH] drm/bridge: panel: Check device dependency before managing device link
On Mon, Nov 27, 2023 at 2:36 PM Ville Syrjälä wrote: > > + panel_bridge->is_independent = !device_is_dependent(drm_dev->dev, > > + panel->dev); > > This broke the build. Looks like device_is_dependent() is not even exported. > ERROR: modpost: "device_is_dependent" [drivers/gpu/drm/drm_kms_helper.ko] > undefined! Yep Ying made a fix, I just applied and pushed it because it was obviously correct. > The recommended defconfigs in rerere-cache do seem to have > CONFIG_DRM_KMS_HELPER=m > so this should have been caught before pushing. How did it slip through? My neglect. Neglect may have something to do with my desktop being 10 years+ old. Building random defconfigs on this machine takes so long time that invariably other stuff has been pushed to the branch before the compiles even complete. Yeah I know. I should buy a better computer. Yours, Linus Walleij
