RE: [Intel-gfx] [PATCH v6 07/10] drm/i915/hdcp: Use HDCP helpers for i915
>
> From: Sean Paul
>
> Now that all of the HDCP 1.x logic has been migrated to the central HDCP
> helpers, use it in the i915 driver.
>
> The majority of the driver code for HDCP 1.x will live in intel_hdcp.c,
> however there are a few helper hooks which are connector-specific and
> need to be partially or fully implemented in the intel_dp_hdcp.c or
> intel_hdmi.c.
>
> We'll leave most of the HDCP 2.x code alone since we don't have another
> implementation of HDCP 2.x to use as reference for what should and
> should not live in the drm helpers. The helper will call the overly
> general enable/disable/is_capable HDCP 2.x callbacks and leave the
> interesting stuff for the driver. Once we have another HDCP 2.x
> implementation, we should do a similar migration.
>
> Acked-by: Jani Nikula
> Signed-off-by: Sean Paul
> Signed-off-by: Mark Yacoub
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-8-
> s...@poorly.run #v1
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-8-
> s...@poorly.run #v2
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-8-
> s...@poorly.run #v3
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-
> 8-s...@poorly.run #v4
> Link:
> https://patchwork.freedesktop.org/patch/msgid/20220411204741.1074308-
> 8-s...@poorly.run #v5
>
> Changes in v2:
> -Fix mst helper function pointer reported by 0-day
> Changes in v3:
> -Add forward declaration for drm_atomic_state in intel_hdcp.h identified
> by 0-day
> Changes in v4:
> -None
> Changes in v5:
> -None
> Changes in v6:
> -Rebased.
>
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 32 +-
> .../drm/i915/display/intel_display_debugfs.c | 6 +-
> .../drm/i915/display/intel_display_types.h| 60 +-
> drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 348 +++
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 +-
> drivers/gpu/drm/i915/display/intel_hdcp.c | 952 +++---
> drivers/gpu/drm/i915/display/intel_hdcp.h | 31 +-
> drivers/gpu/drm/i915/display/intel_hdmi.c | 270 ++---
> 8 files changed, 445 insertions(+), 1270 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 69ecf2a3d6c6..a4397f066a3e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -28,6 +28,7 @@
> #include
>
> #include
> +#include
> #include
>
> #include "i915_drv.h"
> @@ -2909,6 +2910,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
>const struct intel_crtc_state *crtc_state,
>const struct drm_connector_state *conn_state)
> {
> + struct intel_connector *connector =
> + to_intel_connector(conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
>
> if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> @@ -2925,12 +2930,10 @@ static void intel_enable_ddi(struct
> intel_atomic_state *state,
> else
> intel_enable_ddi_dp(state, encoder, crtc_state,
> conn_state);
>
> - /* Enable hdcp if it's desired */
> - if (conn_state->content_protection ==
> - DRM_MODE_CONTENT_PROTECTION_DESIRED)
> - intel_hdcp_enable(to_intel_connector(conn_state-
> >connector),
> - crtc_state,
> - (u8)conn_state->hdcp_content_type);
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> + >base,
> + _port->hdcp_mutex);
> }
>
> static void intel_disable_ddi_dp(struct intel_atomic_state *state,
> @@ -2976,7 +2979,14 @@ static void intel_disable_ddi(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state
> *old_conn_state)
> {
> - intel_hdcp_disable(to_intel_connector(old_conn_state-
> >connector));
> + struct intel_connector *connector =
> + to_intel_connector(old_conn_state->connector);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +
> + if (connector->hdcp_helper_data)
> + drm_hdcp_helper_atomic_commit(connector-
> >hdcp_helper_data,
> + >base,
> + _port->hdcp_mutex);
>
> if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
> intel_disable_ddi_hdmi(state, encoder, old_crtc_state,
> @@ -3004,13 +3014,19 @@ void intel_ddi_update_pipe(struct
> intel_atomic_state *state,
> const struct intel_crtc_state *crtc_state,
>
[PATCH v2] drm/bridge: Fix returned array size name for atomic_get_input_bus_fmts kdoc
The returned array size for input formats is set through
atomic_get_input_bus_fmts()'s 'num_input_fmts' argument, so use
'num_input_fmts' to represent the array size in the function's kdoc,
not 'num_output_fmts'.
Fixes: 91ea83306bfa ("drm/bridge: Fix the bridge kernel doc")
Fixes: f32df58acc68 ("drm/bridge: Add the necessary bits to support bus format
negotiation")
Signed-off-by: Liu Ying
---
v1->v2:
* Correct Fixes tag format.
* Copy DRM bridge driver maintainers.
* Copy Boris.
include/drm/drm_bridge.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 42f86327b40a..bf964cdfb330 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -423,11 +423,11 @@ struct drm_bridge_funcs {
*
* The returned array must be allocated with kmalloc() and will be
* freed by the caller. If the allocation fails, NULL should be
-* returned. num_output_fmts must be set to the returned array size.
+* returned. num_input_fmts must be set to the returned array size.
* Formats listed in the returned array should be listed in decreasing
* preference order (the core will try all formats until it finds one
* that works). When the format is not supported NULL should be
-* returned and num_output_fmts should be set to 0.
+* returned and num_input_fmts should be set to 0.
*
* This method is called on all elements of the bridge chain as part of
* the bus format negotiation process that happens in
--
2.37.1
[PATCH -next 2/3] video: fbdev: wm8505fb: Use devm_platform_ioremap_resource()
According to commit 7945f929f1a7 ("drivers: provide
devm_platform_ioremap_resource()"), convert platform_get_resource(),
devm_ioremap_resource() to a single call to Use
devm_platform_ioremap_resource(), as this is exactly what this function
does.
Signed-off-by: Yang Li
---
drivers/video/fbdev/wm8505fb.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/wm8505fb.c b/drivers/video/fbdev/wm8505fb.c
index 8f4d674fa0d0..96a6f7623e19 100644
--- a/drivers/video/fbdev/wm8505fb.c
+++ b/drivers/video/fbdev/wm8505fb.c
@@ -261,7 +261,6 @@ static const struct fb_ops wm8505fb_ops = {
static int wm8505fb_probe(struct platform_device *pdev)
{
struct wm8505fb_info*fbi;
- struct resource *res;
struct display_timings *disp_timing;
void*addr;
int ret;
@@ -299,8 +298,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
addr = addr + sizeof(struct wm8505fb_info);
fbi->fb.pseudo_palette = addr;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- fbi->regbase = devm_ioremap_resource(>dev, res);
+ fbi->regbase = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(fbi->regbase))
return PTR_ERR(fbi->regbase);
--
2.20.1.7.g153144c
[PATCH -next 3/3] video: fbdev: xilinxfb: Use devm_platform_get_and_ioremap_resource()
According to commit 890cc39a8799 ("drivers: provide
devm_platform_get_and_ioremap_resource()"), convert
platform_get_resource(), devm_ioremap_resource() to a single
call to devm_platform_get_and_ioremap_resource(), as this is exactly
what this function does.
Signed-off-by: Yang Li
---
drivers/video/fbdev/xilinxfb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/xilinxfb.c b/drivers/video/fbdev/xilinxfb.c
index c17cfffd9a84..7911354827dc 100644
--- a/drivers/video/fbdev/xilinxfb.c
+++ b/drivers/video/fbdev/xilinxfb.c
@@ -273,8 +273,7 @@ static int xilinxfb_assign(struct platform_device *pdev,
if (drvdata->flags & BUS_ACCESS_FLAG) {
struct resource *res;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- drvdata->regs = devm_ioremap_resource(>dev, res);
+ drvdata->regs = devm_platform_get_and_ioremap_resource(pdev, 0,
);
if (IS_ERR(drvdata->regs))
return PTR_ERR(drvdata->regs);
--
2.20.1.7.g153144c
[PATCH -next 1/3] video: fbdev: pxa3xx-gcu: Use devm_platform_get_and_ioremap_resource()
According to commit 890cc39a8799 ("drivers: provide
devm_platform_get_and_ioremap_resource()"), convert
platform_get_resource(), devm_ioremap_resource() to a single
call to devm_platform_get_and_ioremap_resource(), as this is exactly
what this function does.
Signed-off-by: Yang Li
---
drivers/video/fbdev/pxa3xx-gcu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c
index c3cd1e1cc01b..d16729215423 100644
--- a/drivers/video/fbdev/pxa3xx-gcu.c
+++ b/drivers/video/fbdev/pxa3xx-gcu.c
@@ -599,8 +599,7 @@ static int pxa3xx_gcu_probe(struct platform_device *pdev)
priv->misc_dev.fops = _gcu_miscdev_fops;
/* handle IO resources */
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- priv->mmio_base = devm_ioremap_resource(dev, r);
+ priv->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, );
if (IS_ERR(priv->mmio_base))
return PTR_ERR(priv->mmio_base);
--
2.20.1.7.g153144c
[PATCH] drm/lima/lima_drv: Add missing unwind goto in lima_pdev_probe()
Smatch reports:
drivers/gpu/drm/lima/lima_drv.c:396 lima_pdev_probe() warn:
missing unwind goto?
Store return value in err and goto 'err_out0' which has
lima_sched_slab_fini() before returning.
Fixes: a1d2a6339961 ("drm/lima: driver for ARM Mali4xx GPUs")
Signed-off-by: Harshit Mogalapalli
---
Only compile tested.
---
drivers/gpu/drm/lima/lima_drv.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
index 7b8d7178d09a..39cab4a55f57 100644
--- a/drivers/gpu/drm/lima/lima_drv.c
+++ b/drivers/gpu/drm/lima/lima_drv.c
@@ -392,8 +392,10 @@ static int lima_pdev_probe(struct platform_device *pdev)
/* Allocate and initialize the DRM device. */
ddev = drm_dev_alloc(_drm_driver, >dev);
- if (IS_ERR(ddev))
- return PTR_ERR(ddev);
+ if (IS_ERR(ddev)) {
+ err = PTR_ERR(ddev);
+ goto err_out0;
+ }
ddev->dev_private = ldev;
ldev->ddev = ddev;
--
2.38.1
Re: [PATCH v5 30/32] drm/msm/dpu: drop smart_dma_rev from dpu_caps
On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote: The code doesn't use dpu_caps::smart_dma_rev field. It checks if the corresponding feature is enabled in the SSPP features. Drop the smart_dma_rev field completely. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 29/32] drm/msm/dpu: enable SmartDMA for the rest of the platforms
On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote:
Enable SmartDMA features for the rest of the platforms where it is
supposed to work.
Signed-off-by: Dmitry Baryshkov
I am so glad we split this. Without visual validation check we wouldnt
have caught the issues and would have ended up with a blank half screen
on our sc7280 CBs on DP.
For sc7280, I validated the foll cases:
1) Basic Chrome UI comes up
2) Validated some browser use-cases and played some youtube videos
3) Validated multiple plug-in/plug-out cases with DP connected
4) IGT test cases with DP connected:
- kms_atomic
- kms_atomic_transition
- kms_pipe_basic_crc
Some notes:
1) I wanted to test 4k with this too but my monitor only supports 4k@60
which is not possible with 24bpp with 2 lanes and due to the HBR3 black
screen issue I could not test that so far. But since I have that issue
even with 1080P and without these changes, it should have no effect due
to this series.
2) I wanted to test some YUV overlay cases but I still cannot find one
on chrome. Its always using RGB. Will check with others.
With these two noted, this change and this series has my
Tested-by: Abhinav Kumar
Only for sc7280 device.
I still cannot give my R-b on this change till others validate visually
and ensure things arent broken for them.
If we don't get enough validation and if only sc7280 remains in this
change, please re-post it with only that and I will give my R-b too.
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 54 ---
1 file changed, 23 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 1fc0dfbebb7e..fc818b0273e7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -21,19 +21,16 @@
(VIG_MASK | BIT(DPU_SSPP_SCALER_QSEED3))
#define VIG_SDM845_MASK \
- (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3))
-
-#define VIG_SDM845_MASK_SDMA \
- (VIG_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+ (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3) |\
+ BIT(DPU_SSPP_SMART_DMA_V2))
#define VIG_SC7180_MASK \
- (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4))
+ (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED4) |\
+ BIT(DPU_SSPP_SMART_DMA_V2))
#define VIG_SM8250_MASK \
- (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE))
-
-#define VIG_SM8250_MASK_SDMA \
- (VIG_SM8250_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
+ (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL) | BIT(DPU_SSPP_SCALER_QSEED3LITE) |\
+ BIT(DPU_SSPP_SMART_DMA_V2))
#define VIG_QCM2290_MASK (VIG_MASK | BIT(DPU_SSPP_QOS_8LVL))
@@ -48,17 +45,12 @@
#define DMA_SDM845_MASK \
(BIT(DPU_SSPP_SRC) | BIT(DPU_SSPP_QOS) | BIT(DPU_SSPP_QOS_8LVL) |\
BIT(DPU_SSPP_TS_PREFILL) | BIT(DPU_SSPP_TS_PREFILL_REC1) |\
+ BIT(DPU_SSPP_SMART_DMA_V2) |\
BIT(DPU_SSPP_CDP) | BIT(DPU_SSPP_EXCL_RECT))
#define DMA_CURSOR_SDM845_MASK \
(DMA_SDM845_MASK | BIT(DPU_SSPP_CURSOR))
-#define DMA_SDM845_MASK_SDMA \
- (DMA_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
-
-#define DMA_CURSOR_SDM845_MASK_SDMA \
- (DMA_CURSOR_SDM845_MASK | BIT(DPU_SSPP_SMART_DMA_V2))
-
#define DMA_CURSOR_MSM8998_MASK \
(DMA_MSM8998_MASK | BIT(DPU_SSPP_CURSOR))
@@ -1208,21 +1200,21 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
};
static const struct dpu_sspp_cfg sdm845_sspp[] = {
- SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, VIG_SDM845_MASK,
sdm845_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
- SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_1", SSPP_VIG1, 0x6000, VIG_SDM845_MASK,
sdm845_vig_sblk_1, 4, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG1),
- SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_2", SSPP_VIG2, 0x8000, VIG_SDM845_MASK,
sdm845_vig_sblk_2, 8, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG2),
- SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_3", SSPP_VIG3, 0xa000, VIG_SDM845_MASK,
sdm845_vig_sblk_3, 12, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG3),
- SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK,
sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
- SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK,
sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
- SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK_SDMA,
+ SSPP_BLK("sspp_10", SSPP_DMA2,
Re: [PATCH v5 28/32] drm/msm/dpu: populate SmartDMA features in hw catalog
On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote: Downstream driver uses dpu->caps->smart_dma_rev to update sspp->cap->features with the bit corresponding to the supported SmartDMA version. Upstream driver does not do this, resulting in SSPP subdriver not enabling setup_multirect callback. Add corresponding SmartDMA SSPP feature bits to dpu hw catalog. Per Abhinav's request enable the SmartDMA features only on the platforms where the multirect was actually verified visually (sdm845 and sm8250). An (untested) enablement on the rest of the platforms comes in the next patch. Signed-off-by: Dmitry Baryshkov Thanks for splitting this up. Strictly on the basis of your validation of these chipsets, this is Reviewed-by: Abhinav Kumar
Re: [PATCH v5 32/32] drm/msm/dpu: remove unused dpu_plane_validate_multirect_v2 function
On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote:
From: Abhinav Kumar
After cleaning up the older multirect support the function
dpu_plane_validate_multirect_v2() is unused. Lets remove it.
Signed-off-by: Abhinav Kumar
this needs your signed-off too.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 111 --
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 7 --
2 files changed, 118 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 9a03d1cad0ee..bafa1dd1748b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -707,117 +707,6 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
fill_color, fmt);
}
-int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
-{
- struct dpu_plane_state *pstate[R_MAX];
- const struct drm_plane_state *drm_state[R_MAX];
- struct drm_rect src[R_MAX], dst[R_MAX];
- struct dpu_plane *dpu_plane[R_MAX];
- const struct dpu_format *fmt[R_MAX];
- int i, buffer_lines;
- unsigned int max_tile_height = 1;
- bool parallel_fetch_qualified = true;
- bool has_tiled_rect = false;
-
- for (i = 0; i < R_MAX; i++) {
- const struct msm_format *msm_fmt;
-
- drm_state[i] = i ? plane->r1 : plane->r0;
- msm_fmt = msm_framebuffer_format(drm_state[i]->fb);
- fmt[i] = to_dpu_format(msm_fmt);
-
- if (DPU_FORMAT_IS_UBWC(fmt[i])) {
- has_tiled_rect = true;
- if (fmt[i]->tile_height > max_tile_height)
- max_tile_height = fmt[i]->tile_height;
- }
- }
-
- for (i = 0; i < R_MAX; i++) {
- int width_threshold;
-
- pstate[i] = to_dpu_plane_state(drm_state[i]);
- dpu_plane[i] = to_dpu_plane(drm_state[i]->plane);
-
- if (pstate[i] == NULL) {
- DPU_ERROR("DPU plane state of plane id %d is NULL\n",
- drm_state[i]->plane->base.id);
- return -EINVAL;
- }
-
- src[i].x1 = drm_state[i]->src_x >> 16;
- src[i].y1 = drm_state[i]->src_y >> 16;
- src[i].x2 = src[i].x1 + (drm_state[i]->src_w >> 16);
- src[i].y2 = src[i].y1 + (drm_state[i]->src_h >> 16);
-
- dst[i] = drm_plane_state_dest(drm_state[i]);
-
- if (drm_rect_calc_hscale([i], [i], 1, 1) != 1 ||
- drm_rect_calc_vscale([i], [i], 1, 1) != 1) {
- DPU_ERROR_PLANE(dpu_plane[i],
- "scaling is not supported in multirect mode\n");
- return -EINVAL;
- }
-
- if (DPU_FORMAT_IS_YUV(fmt[i])) {
- DPU_ERROR_PLANE(dpu_plane[i],
- "Unsupported format for multirect mode\n");
- return -EINVAL;
- }
-
- /**
-* SSPP PD_MEM is split half - one for each RECT.
-* Tiled formats need 5 lines of buffering while fetching
-* whereas linear formats need only 2 lines.
-* So we cannot support more than half of the supported SSPP
-* width for tiled formats.
-*/
- width_threshold = dpu_plane[i]->catalog->caps->max_linewidth;
- if (has_tiled_rect)
- width_threshold /= 2;
-
- if (parallel_fetch_qualified &&
- drm_rect_width([i]) > width_threshold)
- parallel_fetch_qualified = false;
-
- }
-
- /* Validate RECT's and set the mode */
-
- /* Prefer PARALLEL FETCH Mode over TIME_MX Mode */
- if (parallel_fetch_qualified) {
- pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
- pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
-
- goto done;
- }
-
- /* TIME_MX Mode */
- buffer_lines = 2 * max_tile_height;
-
- if (dst[R1].y1 >= dst[R0].y2 + buffer_lines ||
- dst[R0].y1 >= dst[R1].y2 + buffer_lines) {
- pstate[R0]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
- pstate[R1]->pipe.multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
- } else {
- DPU_ERROR(
- "No multirect mode possible for the planes (%d - %d)\n",
- drm_state[R0]->plane->base.id,
- drm_state[R1]->plane->base.id);
- return -EINVAL;
- }
-
-done:
- pstate[R0]->pipe.multirect_index = DPU_SSPP_RECT_0;
- pstate[R1]->pipe.multirect_index = DPU_SSPP_RECT_1;
-
- DPU_DEBUG_PLANE(dpu_plane[R0], "R0: %d -
Re: [PATCH v5 31/32] drm/msm/dpu: log the multirect_index in _dpu_crtc_blend_setup_pipe
On 3/9/2023 4:57 PM, Dmitry Baryshkov wrote:
From: Abhinav Kumar
Lets print the multirect_index as well in _dpu_crtc_blend_setup_pipe()
as it will give the complete information of the sw_pipe as well.
Signed-off-by: Abhinav Kumar
This needs your signed-off too
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b748c4f17c90..96ffea069120 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -425,12 +425,13 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc
*crtc,
format->base.pixel_format,
modifier);
- DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+ DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx
%d\n",
crtc->base.id,
stage,
plane->base.id,
sspp_idx - SSPP_NONE,
-state->fb ? state->fb->base.id : -1);
+state->fb ? state->fb->base.id : -1,
+pipe->multirect_index);
stage_cfg->stage[stage][stage_idx] = sspp_idx;
stage_cfg->multirect_index[stage][stage_idx] = pipe->multirect_index;
Re: [PATCH v5 16/32] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
On 3/13/2023 9:14 PM, Dmitry Baryshkov wrote: On 14/03/2023 06:02, Abhinav Kumar wrote: On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: Move plane state updates from dpu_crtc_atomic_check() to the function where they belong: to dpu_plane_atomic_check(). Signed-off-by: Dmitry Baryshkov Can you please elaborate a bit on this comment? https://patchwork.freedesktop.org/patch/521356/?series=99909=4#comment_949772 Is something still missing? I dont see the multirects being cleared in dpu_plane_atomic_disable() before or even now. It is done in the last patch, where it belongs. There is no need to clear multirect setup until we support RECT_1. Yup, checked the relevant patch (it wasnt the last). Reviewed-by: Abhinav Kumar
Re: [PATCH v5 27/32] drm/msm/dpu: add support for wide planes
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote:
Typically SSPP can support rectangle with width up to 2560. However it's
Please change 2560 > max line width of SSPP as previously commented
possible to use multirect feature and split source to use the SSPP to
output two consecutive rectangles. This commit brings in this capability
to support wider screen resolutions.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 19 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 129 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +
3 files changed, 135 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e651e4593280..b748c4f17c90 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -480,6 +480,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
format, fb ? fb->modifier : 0,
>pipe, 0, stage_cfg);
+ if (pstate->r_pipe.sspp) {
+ set_bit(pstate->r_pipe.sspp->idx, fetch_active);
+ _dpu_crtc_blend_setup_pipe(crtc, plane,
+ mixer, cstate->num_mixers,
+ pstate->stage,
+ format, fb ? fb->modifier :
0,
+ >r_pipe, 1,
stage_cfg);
+ }
+
/* blend config update */
for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
_dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate,
format);
@@ -1316,10 +1325,16 @@ static int _dpu_debugfs_status_show(struct seq_file *s,
void *data)
seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
state->crtc_x, state->crtc_y, state->crtc_w,
state->crtc_h);
- seq_printf(s, "\tsspp:%s\n",
+ seq_printf(s, "\tsspp[0]:%s\n",
pstate->pipe.sspp->cap->name);
- seq_printf(s, "\tmultirect: mode: %d index: %d\n",
+ seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
pstate->pipe.multirect_mode,
pstate->pipe.multirect_index);
+ if (pstate->r_pipe.sspp) {
+ seq_printf(s, "\tsspp[1]:%s\n",
+ pstate->r_pipe.sspp->cap->name);
+ seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
+ pstate->r_pipe.multirect_mode,
pstate->r_pipe.multirect_index);
+ }
seq_puts(s, "\n");
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 3d798b939faa..9a03d1cad0ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -701,6 +701,10 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
/* update sspp */
_dpu_plane_color_fill_pipe(pstate, >pipe,
>pipe_cfg.dst_rect,
fill_color, fmt);
+
+ if (pstate->r_pipe.sspp)
+ _dpu_plane_color_fill_pipe(pstate, >r_pipe,
>r_pipe_cfg.dst_rect,
+ fill_color, fmt);
}
int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
@@ -958,9 +962,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
int ret = 0, min_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
+ struct dpu_sw_pipe *pipe = >pipe;
+ struct dpu_sw_pipe *r_pipe = >r_pipe;
const struct drm_crtc_state *crtc_state = NULL;
const struct dpu_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
+ struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
struct drm_rect fb_rect = { 0 };
uint32_t max_linewidth;
unsigned int rotation;
@@ -984,8 +991,11 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ r_pipe->sspp = NULL;
pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
@@ -1017,19 +1027,58 @@ static int dpu_plane_atomic_check(struct drm_plane
*plane,
max_linewidth =
[PATCH] drm/amdgpu/nv: Apply ASPM quirk on Intel ADL + AMD Navi
S2idle resume freeze can be observed on Intel ADL + AMD WX5500. This is
caused by commit 0064b0ce85bb ("drm/amd/pm: enable ASPM by default").
The root cause is still not clear for now.
So extend and apply the ASPM quirk from commit e02fe3bc7aba
("drm/amdgpu: vi: disable ASPM on Intel Alder Lake based systems"), to
workaround the issue on Navi cards too.
Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2458
Signed-off-by: Kai-Heng Feng
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++
drivers/gpu/drm/amd/amdgpu/nv.c| 2 +-
drivers/gpu/drm/amd/amdgpu/vi.c| 15 ---
4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 164141bc8b4a..c697580f1ee4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1272,6 +1272,7 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device
*adev);
int amdgpu_device_pci_reset(struct amdgpu_device *adev);
bool amdgpu_device_need_post(struct amdgpu_device *adev);
bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
+bool aspm_support_quirk_check(void);
void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
u64 num_vis_bytes);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c4a4e2fe6681..c09f19385628 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -80,6 +80,10 @@
#include
+#if IS_ENABLED(CONFIG_X86)
+#include
+#endif
+
MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin");
@@ -1356,6 +1360,17 @@ bool amdgpu_device_should_use_aspm(struct amdgpu_device
*adev)
return pcie_aspm_enabled(adev->pdev);
}
+bool aspm_support_quirk_check(void)
+{
+#if IS_ENABLED(CONFIG_X86)
+ struct cpuinfo_x86 *c = _data(0);
+
+ return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
+#else
+ return true;
+#endif
+}
+
/* if we get transitioned to only one device, take VGA back */
/**
* amdgpu_device_vga_set_decode - enable/disable vga decode
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 855d390c41de..921adf66e3c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -578,7 +578,7 @@ static void nv_pcie_gen3_enable(struct amdgpu_device *adev)
static void nv_program_aspm(struct amdgpu_device *adev)
{
- if (!amdgpu_device_should_use_aspm(adev))
+ if (!amdgpu_device_should_use_aspm(adev) || !aspm_support_quirk_check())
return;
if (!(adev->flags & AMD_IS_APU) &&
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 12ef782eb478..e61ae372d674 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -81,10 +81,6 @@
#include "mxgpu_vi.h"
#include "amdgpu_dm.h"
-#if IS_ENABLED(CONFIG_X86)
-#include
-#endif
-
#define ixPCIE_LC_L1_PM_SUBSTATE 0x100100C6
#define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK
0x0001L
#define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK 0x0002L
@@ -1138,17 +1134,6 @@ static void vi_enable_aspm(struct amdgpu_device *adev)
WREG32_PCIE(ixPCIE_LC_CNTL, data);
}
-static bool aspm_support_quirk_check(void)
-{
-#if IS_ENABLED(CONFIG_X86)
- struct cpuinfo_x86 *c = _data(0);
-
- return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
-#else
- return true;
-#endif
-}
-
static void vi_program_aspm(struct amdgpu_device *adev)
{
u32 data, data1, orig;
--
2.34.1
Re: [PATCH v5 26/32] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a separate functon. This is a preparation for the r_pipe support. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 25/32] drm/msm/dpu: rework static color fill code
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: Rework static color fill code to separate the pipe / pipe_cfg handling. This is a preparation for the r_pipe support. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 23/32] drm/msm/dpu: rework dpu_plane_atomic_check()
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: Split pipe-dependent code from dpu_plane_atomic_check() into the separate function dpu_plane_atomic_check_pipe(). This is one of preparational steps to add r_pipe support. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 22/32] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the separate function dpu_plane_sspp_update_pipe(). This is one of preparational steps to add r_pipe support. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 21/32] drm/msm/dpu: simplify dpu_plane_validate_src()
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: The plane's clipped coordinates has already been validated against FB size in the drm_atomic_plane_check(). There is no need to check them again. Remove corresponding checks and inline dpu_plane_validate_src(). Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 16/32] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
On 14/03/2023 06:02, Abhinav Kumar wrote:
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote:
Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().
Signed-off-by: Dmitry Baryshkov
Can you please elaborate a bit on this comment?
https://patchwork.freedesktop.org/patch/521356/?series=99909=4#comment_949772
Is something still missing? I dont see the multirects being cleared in
dpu_plane_atomic_disable() before or even now.
It is done in the last patch, where it belongs. There is no need to
clear multirect setup until we support RECT_1.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 --
3 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0260c4d6ded7..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc
*crtc,
crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
- struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
const struct drm_plane_state *pstate;
struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
- /* get plane state for all drm planes associated with crtc state */
+ /* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
crtc_state) {
struct dpu_plane_state *dpu_pstate =
to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
- int stage;
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc
*crtc,
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
- dpu_plane_clear_multirect(pstate);
-
dst = drm_plane_state_dest(pstate);
if (!drm_rect_intersect(, )) {
DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
DRM_RECT_ARG());
return -E2BIG;
}
-
- /* verify stage setting before using it */
- stage = DPU_STAGE_0 + pstate->normalized_zpos;
- if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
- DPU_ERROR("> %d plane stages assigned\n",
- dpu_kms->catalog->caps->max_mixer_blendstages -
DPU_STAGE_0);
- return -EINVAL;
- }
-
- to_dpu_plane_state(pstate)->stage = stage;
- DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
}
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index ce01a602cbc9..3fba63fbbd78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct dpu_plane
*pdpu,
return 0;
}
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
-{
- struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
int dpu_plane_validate_multirect_v2(struct
dpu_multirect_plane_states *plane)
{
struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct
drm_plane *plane,
if (!new_plane_state->visible)
return 0;
+ pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+ pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+ pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+ if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
+ DPU_ERROR("> %d plane stages assigned\n",
+ pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
+ return -EINVAL;
+ }
+
src.x1 = new_plane_state->src_x >> 16;
src.y1 = new_plane_state->src_y >> 16;
src.x2 = src.x1 + (new_plane_state->src_w >> 16);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 228db401e905..a08b0539513b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -88,12 +88,6 @@ struct drm_plane *dpu_plane_init(struct drm_device
*dev,
*/
int dpu_plane_validate_multirect_v2(struct
dpu_multirect_plane_states *plane);
-/**
- * dpu_plane_clear_multirect - clear
Re: [PATCH v5 20/32] drm/msm/dpu: add dpu_hw_sspp_cfg to dpu_plane_state
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: Now as all accesses to pipe_cfg and pstate have been cleaned, add struct dpu_hw_sspp_cfg to struct dpu_plane_state, so that dpu_plane_atomic_check() and dpu_plane_atomic_update() do not have a chance to disagree about src/dst rectangles (currently dpu_plane_atomic_check() uses unclipped rectangles, while dpu_plane_atomic_update() uses clipped rectangles calculated by drm_atomic_helper_check_plane_state()). Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
Re: [PATCH v5 16/32] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote:
Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().
Signed-off-by: Dmitry Baryshkov
Can you please elaborate a bit on this comment?
https://patchwork.freedesktop.org/patch/521356/?series=99909=4#comment_949772
Is something still missing? I dont see the multirects being cleared in
dpu_plane_atomic_disable() before or even now.
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 --
3 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0260c4d6ded7..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
- struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
const struct drm_plane_state *pstate;
struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
- /* get plane state for all drm planes associated with crtc state */
+ /* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
- int stage;
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
- dpu_plane_clear_multirect(pstate);
-
dst = drm_plane_state_dest(pstate);
if (!drm_rect_intersect(, )) {
DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
DRM_RECT_ARG());
return -E2BIG;
}
-
- /* verify stage setting before using it */
- stage = DPU_STAGE_0 + pstate->normalized_zpos;
- if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
- DPU_ERROR("> %d plane stages assigned\n",
- dpu_kms->catalog->caps->max_mixer_blendstages
- DPU_STAGE_0);
- return -EINVAL;
- }
-
- to_dpu_plane_state(pstate)->stage = stage;
- DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
}
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index ce01a602cbc9..3fba63fbbd78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
return 0;
}
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
-{
- struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
{
struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
+ pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+ pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+ pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+ if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
+ DPU_ERROR("> %d plane stages assigned\n",
+ pdpu->catalog->caps->max_mixer_blendstages -
DPU_STAGE_0);
+ return -EINVAL;
+ }
+
src.x1 = new_plane_state->src_x >> 16;
src.y1 = new_plane_state->src_y >> 16;
src.x2 = src.x1 + (new_plane_state->src_w >> 16);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 228db401e905..a08b0539513b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -88,12 +88,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
*/
int
Re: [PATCH v5 13/32] drm/msm/dpu: rename dpu_hw_sspp_cfg to dpu_sw_pipe_cfg
On 3/9/2023 4:56 PM, Dmitry Baryshkov wrote: As struct dpu_hw_sspp_cfg describes only the source and destination rectangles, it is a software pipe configuration now. Rename it accordingly. Signed-off-by: Dmitry Baryshkov Reviewed-by: Abhinav Kumar
linux-next: build warnings after merge of the drm-misc tree
Hi all,
After merging the drm-misc tree, today's linux-next build (htmldocs)
produced these warnings:
drivers/gpu/drm/scheduler/sched_main.c:738: warning: Excess function parameter
'file_private' description in 'drm_sched_job_add_syncobj_dependency'
drivers/gpu/drm/scheduler/sched_main.c:738: warning: Function parameter or
member 'file' not described in 'drm_sched_job_add_syncobj_dependency'
drivers/gpu/drm/scheduler/sched_main.c:738: warning: Excess function parameter
'file_private' description in 'drm_sched_job_add_syncobj_dependency'
Introduced by commit
c087bbb6d84e ("drm/sched: Create wrapper to add a syncobj dependency to job")
--
Cheers,
Stephen Rothwell
pgpjBSlEhbv4Y.pgp
Description: OpenPGP digital signature
Re: [PATCH v2] drm/i915/guc: Disable PL1 power limit when loading GuC firmware
Hi Ashutosh,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-intel/for-linux-next-fixes
drm/drm-next drm-exynos/exynos-drm-next drm-misc/drm-misc-next linus/master
v6.3-rc2 next-20230310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url:
https://github.com/intel-lab-lkp/linux/commits/Ashutosh-Dixit/drm-i915-guc-Disable-PL1-power-limit-when-loading-GuC-firmware/20230314-075114
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:
https://lore.kernel.org/r/20230313234936.2005565-1-ashutosh.dixit%40intel.com
patch subject: [PATCH v2] drm/i915/guc: Disable PL1 power limit when loading
GuC firmware
config: x86_64-randconfig-a016-20230313
(https://download.01.org/0day-ci/archive/20230314/202303141032.gnwwcyad-...@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/931e5a87acb79926043e557341fb0dfd68a9b88d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Ashutosh-Dixit/drm-i915-guc-Disable-PL1-power-limit-when-loading-GuC-firmware/20230314-075114
git checkout 931e5a87acb79926043e557341fb0dfd68a9b88d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Link:
https://lore.kernel.org/oe-kbuild-all/202303141032.gnwwcyad-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/i915/gt/uc/intel_uc.c:483:6: warning: variable 'pl1en' is
>> used uninitialized whenever 'if' condition is true
>> [-Wsometimes-uninitialized]
if (ret)
^~~
drivers/gpu/drm/i915/gt/uc/intel_uc.c:572:28: note: uninitialized use occurs
here
hwm_power_max_restore(gt, pl1en); /* Restore PL1 limit */
^
drivers/gpu/drm/i915/gt/uc/intel_uc.c:483:2: note: remove the 'if' if its
condition is always false
if (ret)
^~~~
drivers/gpu/drm/i915/gt/uc/intel_uc.c:474:6: warning: variable 'pl1en' is
used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!intel_uc_fw_is_loadable(>fw)) {
^~
drivers/gpu/drm/i915/gt/uc/intel_uc.c:572:28: note: uninitialized use occurs
here
hwm_power_max_restore(gt, pl1en); /* Restore PL1 limit */
^
drivers/gpu/drm/i915/gt/uc/intel_uc.c:474:2: note: remove the 'if' if its
condition is always false
if (!intel_uc_fw_is_loadable(>fw)) {
^
drivers/gpu/drm/i915/gt/uc/intel_uc.c:464:26: note: initialize the variable
'pl1en' to silence this warning
int ret, attempts, pl1en;
^
= 0
2 warnings generated.
vim +483 drivers/gpu/drm/i915/gt/uc/intel_uc.c
afd088ac05f120 drivers/gpu/drm/i915/gt/uc/intel_uc.c John Harrison
2022-01-06 457
6fbeda0bfd210f drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2020-01-10 458 static int __uc_init_hw(struct intel_uc *uc)
6fbeda0bfd210f drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2020-01-10 459 {
2f8c06cb6622b5 drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2023-01-28 460 struct intel_gt *gt = uc_to_gt(uc);
2f8c06cb6622b5 drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2023-01-28 461 struct drm_i915_private *i915 = gt->i915;
6fbeda0bfd210f drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2020-01-10 462 struct intel_guc *guc = >guc;
6fbeda0bfd210f drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2020-01-10 463 struct intel_huc *huc = >huc;
931e5a87acb799 drivers/gpu/drm/i915/gt/uc/intel_uc.c Ashutosh Dixit
2023-03-13 464 int ret, attempts, pl1en;
6fbeda0bfd210f drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2020-01-10 465
6fbeda0bfd210f drivers/gpu/drm/i915/gt/uc/intel_uc.c Michal Wajdeczko
2020-01-10 466 GEM_BUG_ON(!intel_uc_supports_guc(
[PATCH v13 08/10] drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked()
Export drm_gem_shmem_get_pages_sgt_locked() that will be used by virtio-gpu
shrinker during GEM swap-in operation done under the held reservation lock.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++-
include/drm/drm_gem_shmem_helper.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 9e94652a141c..dd333610d175 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -849,7 +849,7 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct
drm_gem_shmem_object *shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
-static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct
drm_gem_shmem_object *shmem)
+struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct
drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
int ret;
@@ -887,6 +887,7 @@ static struct sg_table
*drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_
drm_gem_shmem_put_pages(shmem);
return ERR_PTR(ret);
}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_get_pages_sgt_locked);
/**
* drm_gem_shmem_get_pages_sgt - Pin pages, dma map them, and return a
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index e99f1715514b..61aaacc6cb99 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -144,6 +144,7 @@ void drm_gem_shmem_purge(struct drm_gem_shmem_object
*shmem);
struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object
*shmem);
struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object
*shmem);
+struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct
drm_gem_shmem_object *shmem);
void drm_gem_shmem_print_info(const struct drm_gem_shmem_object *shmem,
struct drm_printer *p, unsigned int indent);
--
2.39.2
[PATCH v13 10/10] drm/panfrost: Switch to generic memory shrinker
Replace Panfrost's custom memory shrinker with a common drm-shmem
memory shrinker.
Tested-by: Steven Price # Firefly-RK3288
Reviewed-by: Steven Price
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/panfrost/Makefile | 1 -
drivers/gpu/drm/panfrost/panfrost_device.h| 4 -
drivers/gpu/drm/panfrost/panfrost_drv.c | 27 ++--
drivers/gpu/drm/panfrost/panfrost_gem.c | 30 ++--
drivers/gpu/drm/panfrost/panfrost_gem.h | 9 --
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 129 --
drivers/gpu/drm/panfrost/panfrost_job.c | 18 ++-
include/drm/drm_gem_shmem_helper.h| 7 -
8 files changed, 47 insertions(+), 178 deletions(-)
delete mode 100644 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
diff --git a/drivers/gpu/drm/panfrost/Makefile
b/drivers/gpu/drm/panfrost/Makefile
index 7da2b3f02ed9..11622e22cf15 100644
--- a/drivers/gpu/drm/panfrost/Makefile
+++ b/drivers/gpu/drm/panfrost/Makefile
@@ -5,7 +5,6 @@ panfrost-y := \
panfrost_device.o \
panfrost_devfreq.o \
panfrost_gem.o \
- panfrost_gem_shrinker.o \
panfrost_gpu.o \
panfrost_job.o \
panfrost_mmu.o \
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index d9ba68cffb77..28f28bbdbda9 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -116,10 +116,6 @@ struct panfrost_device {
atomic_t pending;
} reset;
- struct mutex shrinker_lock;
- struct list_head shrinker_list;
- struct shrinker shrinker;
-
struct panfrost_devfreq pfdevfreq;
};
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index aa292e4a86eb..e29a2e604257 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -169,7 +169,6 @@ panfrost_lookup_bos(struct drm_device *dev,
break;
}
- atomic_inc(>gpu_usecount);
job->mappings[i] = mapping;
}
@@ -394,7 +393,6 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
{
struct panfrost_file_priv *priv = file_priv->driver_priv;
struct drm_panfrost_madvise *args = data;
- struct panfrost_device *pfdev = dev->dev_private;
struct drm_gem_object *gem_obj;
struct panfrost_gem_object *bo;
int ret = 0;
@@ -407,11 +405,15 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
bo = to_panfrost_bo(gem_obj);
+ if (bo->is_heap) {
+ args->retained = 1;
+ goto out_put_object;
+ }
+
ret = dma_resv_lock_interruptible(bo->base.base.resv, NULL);
if (ret)
goto out_put_object;
- mutex_lock(>shrinker_lock);
mutex_lock(>mappings.lock);
if (args->madv == PANFROST_MADV_DONTNEED) {
struct panfrost_gem_mapping *first;
@@ -437,17 +439,8 @@ static int panfrost_ioctl_madvise(struct drm_device *dev,
void *data,
args->retained = drm_gem_shmem_madvise(>base, args->madv);
- if (args->retained) {
- if (args->madv == PANFROST_MADV_DONTNEED)
- list_move_tail(>base.madv_list,
- >shrinker_list);
- else if (args->madv == PANFROST_MADV_WILLNEED)
- list_del_init(>base.madv_list);
- }
-
out_unlock_mappings:
mutex_unlock(>mappings.lock);
- mutex_unlock(>shrinker_lock);
dma_resv_unlock(bo->base.base.resv);
out_put_object:
drm_gem_object_put(gem_obj);
@@ -579,9 +572,6 @@ static int panfrost_probe(struct platform_device *pdev)
ddev->dev_private = pfdev;
pfdev->ddev = ddev;
- mutex_init(>shrinker_lock);
- INIT_LIST_HEAD(>shrinker_list);
-
err = panfrost_device_init(pfdev);
if (err) {
if (err != -EPROBE_DEFER)
@@ -603,10 +593,14 @@ static int panfrost_probe(struct platform_device *pdev)
if (err < 0)
goto err_out1;
- panfrost_gem_shrinker_init(ddev);
+ err = drmm_gem_shmem_init(ddev);
+ if (err < 0)
+ goto err_out2;
return 0;
+err_out2:
+ drm_dev_unregister(ddev);
err_out1:
pm_runtime_disable(pfdev->dev);
panfrost_device_fini(pfdev);
@@ -622,7 +616,6 @@ static int panfrost_remove(struct platform_device *pdev)
struct drm_device *ddev = pfdev->ddev;
drm_dev_unregister(ddev);
- panfrost_gem_shrinker_cleanup(ddev);
pm_runtime_get_sync(pfdev->dev);
pm_runtime_disable(pfdev->dev);
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c
b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 3c812fbd126f..08d795c28b4e 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++
[PATCH v13 09/10] drm/virtio: Support memory shrinking
Support generic drm-shmem memory shrinker and add new madvise IOCTL to
the VirtIO-GPU driver. BO cache manager of Mesa driver will mark BOs as
"don't need" using the new IOCTL to let shrinker purge the marked BOs on
OOM, the shrinker will also evict unpurgeable shmem BOs from memory if
guest supports SWAP file or partition.
Acked-by: Gerd Hoffmann
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/virtio/virtgpu_drv.h| 20 +++-
drivers/gpu/drm/virtio/virtgpu_gem.c| 68
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 37 +++
drivers/gpu/drm/virtio/virtgpu_kms.c| 8 ++
drivers/gpu/drm/virtio/virtgpu_object.c | 137 +++-
drivers/gpu/drm/virtio/virtgpu_plane.c | 17 ++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 40 +++
include/uapi/drm/virtgpu_drm.h | 14 +++
8 files changed, 310 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index af6ffb696086..589c95822699 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -89,6 +89,7 @@ struct virtio_gpu_object {
uint32_t hw_res_handle;
bool dumb;
bool created;
+ bool detached;
bool host3d_blob, guest_blob;
uint32_t blob_mem, blob_flags;
@@ -277,7 +278,7 @@ struct virtio_gpu_fpriv {
};
/* virtgpu_ioctl.c */
-#define DRM_VIRTIO_NUM_IOCTLS 12
+#define DRM_VIRTIO_NUM_IOCTLS 13
extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
@@ -313,6 +314,12 @@ void virtio_gpu_array_put_free(struct
virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_delayed(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
void virtio_gpu_array_put_free_work(struct work_struct *work);
+int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_object_array *objs);
+int virtio_gpu_gem_host_mem_release(struct virtio_gpu_object *bo);
+int virtio_gpu_gem_madvise(struct virtio_gpu_object *obj, int madv);
+int virtio_gpu_gem_pin(struct virtio_gpu_object *bo);
+void virtio_gpu_gem_unpin(struct virtio_gpu_object *bo);
/* virtgpu_vq.c */
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev);
@@ -324,6 +331,8 @@ void virtio_gpu_cmd_create_resource(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
+int virtio_gpu_cmd_release_resource(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *bo);
void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
uint64_t offset,
uint32_t width, uint32_t height,
@@ -344,6 +353,9 @@ void virtio_gpu_object_attach(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_object *obj,
struct virtio_gpu_mem_entry *ents,
unsigned int nents);
+void virtio_gpu_object_detach(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_object *obj,
+ struct virtio_gpu_fence *fence);
int virtio_gpu_attach_status_page(struct virtio_gpu_device *vgdev);
int virtio_gpu_detach_status_page(struct virtio_gpu_device *vgdev);
void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev,
@@ -456,6 +468,8 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
+int virtio_gpu_reattach_shmem_object(struct virtio_gpu_object *bo);
+
int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
uint32_t *resid);
/* virtgpu_prime.c */
@@ -486,4 +500,8 @@ void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
struct sg_table *sgt,
enum dma_data_direction dir);
+/* virtgpu_gem_shrinker.c */
+int virtio_gpu_gem_shrinker_init(struct virtio_gpu_device *vgdev);
+void virtio_gpu_gem_shrinker_fini(struct virtio_gpu_device *vgdev);
+
#endif
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c
b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 7db48d17ee3a..0f6480b60b68 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -294,3 +294,71 @@ void virtio_gpu_array_put_free_work(struct work_struct
*work)
}
spin_unlock(>obj_free_lock);
}
+
+int virtio_gpu_array_prepare(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_object_array *objs)
+{
+ struct virtio_gpu_object *bo;
+ int ret = 0;
+ u32 i;
+
[PATCH v13 06/10] drm/shmem-helper: Add memory shrinker
Introduce common drm-shmem shrinker for DRM drivers.
To start using drm-shmem shrinker drivers should do the following:
1. Implement evict() callback of GEM object where driver should check
whether object is purgeable or evictable using drm-shmem helpers and
perform the shrinking action
2. Initialize drm-shmem internals using drmm_gem_shmem_init(drm_device),
which will register drm-shmem shrinker
3. Implement madvise IOCTL that will use drm_gem_shmem_madvise()
Signed-off-by: Daniel Almeida
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 351 +-
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 9 +-
include/drm/drm_device.h | 10 +-
include/drm/drm_gem_shmem_helper.h| 52 ++-
4 files changed, 402 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 48df4e87da26..a02377a5131b 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -88,8 +89,6 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
if (ret)
goto err_release;
- INIT_LIST_HEAD(>madv_list);
-
if (!private) {
/*
* Our buffers are kept pinned, so allocating them
@@ -128,6 +127,57 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct
drm_device *dev, size_t
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
+static void drm_gem_shmem_resv_assert_held(struct drm_gem_shmem_object *shmem)
+{
+ /*
+* Destroying the object is a special case.. drm_gem_shmem_free()
+* calls many things that WARN_ON if the obj lock is not held. But
+* acquiring the obj lock in drm_gem_shmem_free() can cause a locking
+* order inversion between reservation_ww_class_mutex and fs_reclaim.
+*
+* This deadlock is not actually possible, because no one should
+* be already holding the lock when drm_gem_shmem_free() is called.
+* Unfortunately lockdep is not aware of this detail. So when the
+* refcount drops to zero, we pretend it is already locked.
+*/
+ if (kref_read(>base.refcount))
+ dma_resv_assert_held(shmem->base.resv);
+}
+
+static bool drm_gem_shmem_is_evictable(struct drm_gem_shmem_object *shmem)
+{
+ dma_resv_assert_held(shmem->base.resv);
+
+ return (shmem->madv >= 0) && shmem->base.funcs->evict &&
+ shmem->pages_use_count && !shmem->pages_pin_count &&
+ !shmem->base.dma_buf && !shmem->base.import_attach &&
+ shmem->sgt && !shmem->evicted;
+}
+
+static void
+drm_gem_shmem_update_pages_state(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = >base;
+ struct drm_gem_shmem *shmem_mm = obj->dev->shmem_mm;
+ struct drm_gem_shmem_shrinker *shmem_shrinker = _mm->shrinker;
+
+ drm_gem_shmem_resv_assert_held(shmem);
+
+ if (!shmem_shrinker || obj->import_attach)
+ return;
+
+ if (shmem->madv < 0)
+ drm_gem_lru_remove(>base);
+ else if (drm_gem_shmem_is_evictable(shmem) ||
drm_gem_shmem_is_purgeable(shmem))
+ drm_gem_lru_move_tail(_shrinker->lru_evictable,
>base);
+ else if (shmem->evicted)
+ drm_gem_lru_move_tail(_shrinker->lru_evicted,
>base);
+ else if (!shmem->pages)
+ drm_gem_lru_remove(>base);
+ else
+ drm_gem_lru_move_tail(_shrinker->lru_pinned,
>base);
+}
+
/**
* drm_gem_shmem_free - Free resources associated with a shmem GEM object
* @shmem: shmem GEM object to free
@@ -142,7 +192,8 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
if (obj->import_attach) {
drm_prime_gem_destroy(obj, shmem->sgt);
} else {
- dma_resv_lock(shmem->base.resv, NULL);
+ /* take out shmem GEM object from the memory shrinker */
+ drm_gem_shmem_madvise(shmem, -1);
drm_WARN_ON(obj->dev, shmem->vmap_use_count);
@@ -152,12 +203,10 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
}
- if (shmem->pages)
+ if (shmem->pages_use_count)
drm_gem_shmem_put_pages(shmem);
drm_WARN_ON(obj->dev, shmem->pages_use_count);
-
- dma_resv_unlock(shmem->base.resv);
}
drm_gem_object_release(obj);
@@ -178,6 +227,11 @@ drm_gem_shmem_acquire_pages(struct drm_gem_shmem_object
*shmem)
return -ENOMEM;
}
+ if (shmem->pages) {
+ drm_WARN_ON(obj->dev, !shmem->evicted);
+ return 0;
+ }
+
if
[PATCH v13 07/10] drm/shmem-helper: Remove obsoleted is_iomem test
Everything that uses the mapped buffer should by agnostic to is_iomem.
The only reason for the is_iomem test is is that we're setting shmem->vaddr
to the returned map->vaddr. Now that the shmem->vaddr code is gone, remove
the obsoleted is_iomem test to clean up the code.
Suggested-by: Thomas Zimmermann
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index a02377a5131b..9e94652a141c 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -432,12 +432,6 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
if (obj->import_attach) {
ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
- if (!ret) {
- if (drm_WARN_ON(obj->dev, map->is_iomem)) {
- dma_buf_vunmap(obj->import_attach->dmabuf, map);
- return -EIO;
- }
- }
} else {
pgprot_t prot = PAGE_KERNEL;
--
2.39.2
[PATCH v13 05/10] drm/shmem-helper: Factor out unpinning part from drm_gem_shmem_purge()
Factor out pages unpinning code from drm_gem_shmem_purge() into new
drm_gem_shmem_unpin_pages(). This prepares code for addition of memory
shrinker support. The new common function will be used by shrinker for
eviction of shmem pages.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 1fcb7d850cc7..48df4e87da26 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -486,25 +486,29 @@ int drm_gem_shmem_madvise(struct drm_gem_shmem_object
*shmem, int madv)
}
EXPORT_SYMBOL(drm_gem_shmem_madvise);
-void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
+static void drm_gem_shmem_unpin_pages(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
struct drm_device *dev = obj->dev;
dma_resv_assert_held(shmem->base.resv);
- drm_WARN_ON(obj->dev, !drm_gem_shmem_is_purgeable(shmem));
-
dma_unmap_sgtable(dev->dev, shmem->sgt, DMA_BIDIRECTIONAL, 0);
+ drm_gem_shmem_release_pages(shmem);
+ drm_vma_node_unmap(>vma_node, dev->anon_inode->i_mapping);
+
sg_free_table(shmem->sgt);
kfree(shmem->sgt);
shmem->sgt = NULL;
+}
- drm_gem_shmem_put_pages(shmem);
+void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = >base;
- shmem->madv = -1;
+ drm_WARN_ON(obj->dev, !drm_gem_shmem_is_purgeable(shmem));
- drm_vma_node_unmap(>vma_node, dev->anon_inode->i_mapping);
+ drm_gem_shmem_unpin_pages(shmem);
drm_gem_free_mmap_offset(obj);
/* Our goal here is to return as much of the memory as
@@ -515,6 +519,8 @@ void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
shmem_truncate_range(file_inode(obj->filp), 0, (loff_t)-1);
invalidate_mapping_pages(file_inode(obj->filp)->i_mapping, 0,
(loff_t)-1);
+
+ shmem->madv = -1;
}
EXPORT_SYMBOL(drm_gem_shmem_purge);
--
2.39.2
[PATCH v13 03/10] drm/shmem-helper: Add pages_pin_count field
And new pages_pin_count field to struct drm_gem_shmem_object that will
determine whether pages are evictable by memory shrinker. The pages will
be evictable only when pages_pin_count=0. This patch prepares code for
addition of the memory shrinker that will utilize the new field.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 7 +++
include/drm/drm_gem_shmem_helper.h | 9 +
2 files changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 4da9c9c39b9a..81d61791f874 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -277,6 +277,8 @@ static int drm_gem_shmem_pin_locked(struct
drm_gem_shmem_object *shmem)
drm_WARN_ON(obj->dev, obj->import_attach);
ret = drm_gem_shmem_get_pages(shmem);
+ if (!ret)
+ shmem->pages_pin_count++;
return ret;
}
@@ -289,7 +291,12 @@ static void drm_gem_shmem_unpin_locked(struct
drm_gem_shmem_object *shmem)
drm_WARN_ON(obj->dev, obj->import_attach);
+ if (drm_WARN_ON_ONCE(obj->dev, !shmem->pages_pin_count))
+ return;
+
drm_gem_shmem_put_pages(shmem);
+
+ shmem->pages_pin_count--;
}
/**
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index 20ddcd799df9..7d823c9fc480 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -39,6 +39,15 @@ struct drm_gem_shmem_object {
*/
unsigned int pages_use_count;
+ /**
+* @pages_pin_count:
+*
+* Reference count on the pinned pages table.
+* The pages allowed to be evicted by memory shrinker
+* only when the count is zero.
+*/
+ unsigned int pages_pin_count;
+
/**
* @madv: State for madvise
*
--
2.39.2
[PATCH v13 04/10] drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin
The vmapped pages shall be pinned in memory. Previously get/put pages were implicitly pinning/unpinning the pages. This will no longer be the case with addition of memory shrinker because pages_use_count>0 won't determine whether pages are pinned anymore, while the new pages_pin_count will do that. Switch the vmap/vunmap to use pin/unpin functions in a preparation of addition of the memory shrinker support. Signed-off-by: Dmitry Osipenko --- drivers/gpu/drm/drm_gem_shmem_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 81d61791f874..1fcb7d850cc7 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -380,7 +380,7 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, return 0; } - ret = drm_gem_shmem_get_pages(shmem); + ret = drm_gem_shmem_pin_locked(shmem); if (ret) goto err_zero_use; @@ -403,7 +403,7 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem, err_put_pages: if (!obj->import_attach) - drm_gem_shmem_put_pages(shmem); + drm_gem_shmem_unpin_locked(shmem); err_zero_use: shmem->vmap_use_count = 0; @@ -440,7 +440,7 @@ void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem, return; vunmap(shmem->vaddr); - drm_gem_shmem_put_pages(shmem); + drm_gem_shmem_unpin_locked(shmem); } shmem->vaddr = NULL; -- 2.39.2
[PATCH v13 01/10] drm/shmem-helper: Switch to reservation lock
Replace all drm-shmem locks with a GEM reservation lock. This makes locks
consistent with dma-buf locking convention where importers are responsible
for holding reservation lock for all operations performed over dma-bufs,
preventing deadlock between dma-buf importers and exporters.
Suggested-by: Daniel Vetter
Acked-by: Thomas Zimmermann
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c| 217 --
drivers/gpu/drm/lima/lima_gem.c | 8 +-
drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +-
.../gpu/drm/panfrost/panfrost_gem_shrinker.c | 6 +-
drivers/gpu/drm/panfrost/panfrost_mmu.c | 19 +-
include/drm/drm_gem_shmem_helper.h| 14 +-
6 files changed, 120 insertions(+), 151 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 4ea6507a77e5..8fc2a3277486 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -88,8 +88,6 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
if (ret)
goto err_release;
- mutex_init(>pages_lock);
- mutex_init(>vmap_lock);
INIT_LIST_HEAD(>madv_list);
if (!private) {
@@ -141,11 +139,13 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
{
struct drm_gem_object *obj = >base;
- drm_WARN_ON(obj->dev, shmem->vmap_use_count);
-
if (obj->import_attach) {
drm_prime_gem_destroy(obj, shmem->sgt);
} else {
+ dma_resv_lock(shmem->base.resv, NULL);
+
+ drm_WARN_ON(obj->dev, shmem->vmap_use_count);
+
if (shmem->sgt) {
dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
DMA_BIDIRECTIONAL, 0);
@@ -154,18 +154,18 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
}
if (shmem->pages)
drm_gem_shmem_put_pages(shmem);
- }
- drm_WARN_ON(obj->dev, shmem->pages_use_count);
+ drm_WARN_ON(obj->dev, shmem->pages_use_count);
+
+ dma_resv_unlock(shmem->base.resv);
+ }
drm_gem_object_release(obj);
- mutex_destroy(>pages_lock);
- mutex_destroy(>vmap_lock);
kfree(shmem);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
-static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
+static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
struct page **pages;
@@ -197,35 +197,16 @@ static int drm_gem_shmem_get_pages_locked(struct
drm_gem_shmem_object *shmem)
}
/*
- * drm_gem_shmem_get_pages - Allocate backing pages for a shmem GEM object
+ * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a
shmem GEM object
* @shmem: shmem GEM object
*
- * This function makes sure that backing pages exists for the shmem GEM object
- * and increases the use count.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
+ * This function decreases the use count and puts the backing pages when use
drops to zero.
*/
-int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
- int ret;
- drm_WARN_ON(obj->dev, obj->import_attach);
-
- ret = mutex_lock_interruptible(>pages_lock);
- if (ret)
- return ret;
- ret = drm_gem_shmem_get_pages_locked(shmem);
- mutex_unlock(>pages_lock);
-
- return ret;
-}
-EXPORT_SYMBOL(drm_gem_shmem_get_pages);
-
-static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
-{
- struct drm_gem_object *obj = >base;
+ dma_resv_assert_held(shmem->base.resv);
if (drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
return;
@@ -243,20 +224,32 @@ static void drm_gem_shmem_put_pages_locked(struct
drm_gem_shmem_object *shmem)
shmem->pages_mark_accessed_on_put);
shmem->pages = NULL;
}
+EXPORT_SYMBOL(drm_gem_shmem_put_pages);
-/*
- * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a
shmem GEM object
- * @shmem: shmem GEM object
- *
- * This function decreases the use count and puts the backing pages when use
drops to zero.
- */
-void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
+static int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem)
{
- mutex_lock(>pages_lock);
- drm_gem_shmem_put_pages_locked(shmem);
- mutex_unlock(>pages_lock);
+ struct drm_gem_object *obj = >base;
+ int ret;
+
+ dma_resv_assert_held(shmem->base.resv);
+
+ drm_WARN_ON(obj->dev, obj->import_attach);
+
+ ret = drm_gem_shmem_get_pages(shmem);
+
+ return ret;
+}
+
+static void
[PATCH v13 02/10] drm/shmem-helper: Factor out pages alloc/release from drm_gem_shmem_get/put_pages()
Factor out pages allocation from drm_gem_shmem_get_pages() into
drm_gem_shmem_acquire_pages() function and similar for the put_pages()
in a preparation for addition of shrinker support to drm-shmem.
Once shrinker will be added, the pages_use_count>0 will no longer determine
whether pages are pinned because pages could be swapped out by the shrinker
and then pages_use_count will be greater than 0 in this case. We will add
new pages_pin_count in a later patch.
The new common drm_gem_shmem_acquire/release_pages() will be used by
shrinker code for performing the page swapping.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 67 +-
1 file changed, 54 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8fc2a3277486..4da9c9c39b9a 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -165,19 +165,26 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object
*shmem)
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
-static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+static int
+drm_gem_shmem_acquire_pages(struct drm_gem_shmem_object *shmem)
{
struct drm_gem_object *obj = >base;
struct page **pages;
- if (shmem->pages_use_count++ > 0)
- return 0;
+ dma_resv_assert_held(shmem->base.resv);
+
+ if (shmem->madv < 0) {
+ drm_WARN_ON(obj->dev, shmem->pages);
+ return -ENOMEM;
+ }
+
+ if (drm_WARN_ON(obj->dev, !shmem->pages_use_count))
+ return -EINVAL;
pages = drm_gem_get_pages(obj);
if (IS_ERR(pages)) {
drm_dbg_kms(obj->dev, "Failed to get pages (%ld)\n",
PTR_ERR(pages));
- shmem->pages_use_count = 0;
return PTR_ERR(pages);
}
@@ -196,6 +203,48 @@ static int drm_gem_shmem_get_pages(struct
drm_gem_shmem_object *shmem)
return 0;
}
+static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
+{
+ int err;
+
+ dma_resv_assert_held(shmem->base.resv);
+
+ if (shmem->madv < 0)
+ return -ENOMEM;
+
+ if (shmem->pages_use_count++ > 0)
+ return 0;
+
+ err = drm_gem_shmem_acquire_pages(shmem);
+ if (err)
+ goto err_zero_use;
+
+ return 0;
+
+err_zero_use:
+ shmem->pages_use_count = 0;
+
+ return err;
+}
+
+static void
+drm_gem_shmem_release_pages(struct drm_gem_shmem_object *shmem)
+{
+ struct drm_gem_object *obj = >base;
+
+ dma_resv_assert_held(shmem->base.resv);
+
+#ifdef CONFIG_X86
+ if (shmem->map_wc)
+ set_pages_array_wb(shmem->pages, obj->size >> PAGE_SHIFT);
+#endif
+
+ drm_gem_put_pages(obj, shmem->pages,
+ shmem->pages_mark_dirty_on_put,
+ shmem->pages_mark_accessed_on_put);
+ shmem->pages = NULL;
+}
+
/*
* drm_gem_shmem_put_pages - Decrease use count on the backing pages for a
shmem GEM object
* @shmem: shmem GEM object
@@ -214,15 +263,7 @@ void drm_gem_shmem_put_pages(struct drm_gem_shmem_object
*shmem)
if (--shmem->pages_use_count > 0)
return;
-#ifdef CONFIG_X86
- if (shmem->map_wc)
- set_pages_array_wb(shmem->pages, obj->size >> PAGE_SHIFT);
-#endif
-
- drm_gem_put_pages(obj, shmem->pages,
- shmem->pages_mark_dirty_on_put,
- shmem->pages_mark_accessed_on_put);
- shmem->pages = NULL;
+ drm_gem_shmem_release_pages(shmem);
}
EXPORT_SYMBOL(drm_gem_shmem_put_pages);
--
2.39.2
[PATCH v13 00/10] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers
This series: 1. Adds common drm-shmem memory shrinker 2. Enables shrinker for VirtIO-GPU driver 3. Switches Panfrost driver to the common shrinker Changelog: v13:- Updated virtio-gpu shrinker patch to use drm_gem_shmem_object_pin() directly instead of drm_gem_pin() and dropped patch that exported drm_gem_pin() functions, like was requested by Thomas Zimmermann in v12. v12:- Fixed the "no previous prototype for function" warning reported by kernel build bot for v11. - Fixed the missing reservation lock reported by Intel CI for VGEM driver. Other drivers using drm-shmem were affected similarly to VGEM. The problem was in the dma-buf attachment code path that led to drm-shmem pinning function which assumed the held reservation lock by drm_gem_pin(). In the past that code path was causing trouble for i915 driver and we've changed the locking scheme for the attachment code path in the dma-buf core to let exporters to handle the locking themselves. After a closer investigation, I realized that my assumption about testing of dma-buf export code path using Panfrost driver was incorrect. Now I created additional local test to exrecise the Panfrost export path. I also reproduced the issue reported by the Intel CI for v10. It's all fixed now by making the drm_gem_shmem_pin() to take the resv lock by itself. - Patches are based on top of drm-tip, CC'd intel-gfx CI for testing. v11:- Rebased on a recent linux-next. Added new patch as a result: drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked() It's needed by the virtio-gpu driver to swap-in/unevict shmem object, previously get_pages_sgt() didn't use locking. - Separated the "Add memory shrinker" patch into smaller parts to ease the reviewing, as was requested by Thomas Zimmermann: drm/shmem-helper: Factor out pages alloc/release from drm_gem_shmem_get/put_pages() drm/shmem-helper: Add pages_pin_count field drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin drm/shmem-helper: Factor out unpinning part from drm_gem_shmem_purge() - Addessed the v10 review comments from Thomas Zimmermann: return errno instead of bool, sort code alphabetically, rename function and etc minor changes. - Added new patch to remove the "map->is_iomem" from drm-shmem, as was suggested by Thomas Zimmermann. - Added acks and r-b's that were given to v10. v10:- Was partially applied to misc-fixes/next. https://lore.kernel.org/dri-devel/6c16f303-81df-7ebe-85e9-51bb40a8b...@collabora.com/T/ Dmitry Osipenko (10): drm/shmem-helper: Switch to reservation lock drm/shmem-helper: Factor out pages alloc/release from drm_gem_shmem_get/put_pages() drm/shmem-helper: Add pages_pin_count field drm/shmem-helper: Switch drm_gem_shmem_vmap/vunmap to use pin/unpin drm/shmem-helper: Factor out unpinning part from drm_gem_shmem_purge() drm/shmem-helper: Add memory shrinker drm/shmem-helper: Remove obsoleted is_iomem test drm/shmem-helper: Export drm_gem_shmem_get_pages_sgt_locked() drm/virtio: Support memory shrinking drm/panfrost: Switch to generic memory shrinker drivers/gpu/drm/drm_gem_shmem_helper.c| 613 ++ drivers/gpu/drm/lima/lima_gem.c | 8 +- drivers/gpu/drm/panfrost/Makefile | 1 - drivers/gpu/drm/panfrost/panfrost_device.h| 4 - drivers/gpu/drm/panfrost/panfrost_drv.c | 34 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 30 +- drivers/gpu/drm/panfrost/panfrost_gem.h | 9 - .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 122 drivers/gpu/drm/panfrost/panfrost_job.c | 18 +- drivers/gpu/drm/panfrost/panfrost_mmu.c | 19 +- drivers/gpu/drm/virtio/virtgpu_drv.h | 20 +- drivers/gpu/drm/virtio/virtgpu_gem.c | 68 ++ drivers/gpu/drm/virtio/virtgpu_ioctl.c| 37 ++ drivers/gpu/drm/virtio/virtgpu_kms.c | 8 + drivers/gpu/drm/virtio/virtgpu_object.c | 137 +++- drivers/gpu/drm/virtio/virtgpu_plane.c| 17 +- drivers/gpu/drm/virtio/virtgpu_vq.c | 40 ++ include/drm/drm_device.h | 10 +- include/drm/drm_gem_shmem_helper.h| 83 ++- include/uapi/drm/virtgpu_drm.h| 14 + 20 files changed, 925 insertions(+), 367 deletions(-) delete mode 100644 drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c -- 2.39.2
Re: [PATCH RFC 03/18] rust: drm: file: Add File abstraction
On Mon, Mar 13, 2023 at 12:49:57PM -0500, Faith Ekstrand wrote:
> On Fri, 2023-03-10 at 07:16 +0900, Asahi Lina wrote:
> > On 10/03/2023 06.16, Faith Ekstrand wrote:
> > > On Tue, 2023-03-07 at 23:25 +0900, Asahi Lina wrote:
> > > > A DRM File is the DRM counterpart to a kernel file structure,
> > > > representing an open DRM file descriptor. Add a Rust abstraction
> > > > to
> > > > allow drivers to implement their own File types that implement
> > > > the
> > > > DriverFile trait.
> > > >
> > > > Signed-off-by: Asahi Lina
> > > > ---
> > > > rust/bindings/bindings_helper.h | 1 +
> > > > rust/kernel/drm/drv.rs | 7 ++-
> > > > rust/kernel/drm/file.rs | 113
> > > >
> > > > rust/kernel/drm/mod.rs | 1 +
> > > > 4 files changed, 120 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/rust/bindings/bindings_helper.h
> > > > b/rust/bindings/bindings_helper.h
> > > > index 2a999138c4ae..7d7828faf89c 100644
> > > > --- a/rust/bindings/bindings_helper.h
> > > > +++ b/rust/bindings/bindings_helper.h
> > > > @@ -8,6 +8,7 @@
> > > >
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
> > > > index 29a465515dc9..1dcb651e1417 100644
> > > > --- a/rust/kernel/drm/drv.rs
> > > > +++ b/rust/kernel/drm/drv.rs
> > > > @@ -144,6 +144,9 @@ pub trait Driver {
> > > > /// Should be either `drm::gem::Object` or
> > > > `drm::gem::shmem::Object`.
> > > > type Object: AllocImpl;
> > > >
> > > > + /// The type used to represent a DRM File (client)
> > > > + type File: drm::file::DriverFile;
> > > > +
> > > > /// Driver metadata
> > > > const INFO: DriverInfo;
> > > >
> > > > @@ -213,8 +216,8 @@ macro_rules! drm_device_register {
> > > > impl Registration {
> > > > const VTABLE: bindings::drm_driver = drm_legacy_fields! {
> > > > load: None,
> > > > - open: None, // TODO: File abstraction
> > > > - postclose: None, // TODO: File abstraction
> > > > + open: Some(drm::file::open_callback::),
> > > > + postclose:
> > > > Some(drm::file::postclose_callback::),
> > > > lastclose: None,
> > > > unload: None,
> > > > release: None,
> > > > diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
> > > > new file mode 100644
> > > > index ..48751e93c38a
> > > > --- /dev/null
> > > > +++ b/rust/kernel/drm/file.rs
> > > > @@ -0,0 +1,113 @@
> > > > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > > > +
> > > > +//! DRM File objects.
> > > > +//!
> > > > +//! C header:
> > > > [`include/linux/drm/drm_file.h`](../../../../include/linux/drm/dr
> > > > m_fi
> > > > le.h)
> > > > +
> > > > +use crate::{bindings, drm, error::Result};
> > > > +use alloc::boxed::Box;
> > > > +use core::marker::PhantomData;
> > > > +use core::ops::Deref;
> > > > +
> > > > +/// Trait that must be implemented by DRM drivers to represent a
> > > > DRM
> > > > File (a client instance).
> > > > +pub trait DriverFile {
> > > > + /// The parent `Driver` implementation for this
> > > > `DriverFile`.
> > > > + type Driver: drm::drv::Driver;
> > > > +
> > > > + /// Open a new file (called when a client opens the DRM
> > > > device).
> > > > + fn open(device: ::device::Device) ->
> > > > Result>;
> > > > +}
> > > > +
> > > > +/// An open DRM File.
> > > > +///
> > > > +/// # Invariants
> > > > +/// `raw` is a valid pointer to a `drm_file` struct.
> > > > +#[repr(transparent)]
> > > > +pub struct File {
> > > > + raw: *mut bindings::drm_file,
> > > > + _p: PhantomData,
> > > > +}
> > > > +
> > > > +pub(super) unsafe extern "C" fn open_callback(
> > > > + raw_dev: *mut bindings::drm_device,
> > > > + raw_file: *mut bindings::drm_file,
> > > > +) -> core::ffi::c_int {
> > > > + let drm = core::mem::ManuallyDrop::new(unsafe {
> > > > drm::device::Device::from_raw(raw_dev) });
> > >
> > > Maybe you can help educate me a bit here... This feels like a
> > > really
> > > sketchy pattern. We're creating a Device from a pointer, an
> > > operation
> > > which inherently consumes a reference but then marking it
> > > ManuallyDrop
> > > so drm_device_put() never gets called. It took me a while but I
> > > think
> > > I figured out what you're trying to do: Make it so all the Rust
> > > stuff
> > > works with Device, not drm_device but it still feels really wrong.
> > > It
> > > works, it just feels like there's a lot of unsafe abstraction
> > > juggling
> > > happening here and I expect this operation is going to be pretty
> > > common
> > > in the Rust abstraction layer.
> >
> > So I think this is going to be a pretty common pattern in this kind
> > of
> > abstraction. The problem is that, of course, in C there is no
> > distinction between an owned reference and a borrowed one. Here we
> > have
> > a borrowed reference to a
Re: [PATCH v15 00/16] drm: Add Samsung MIPI DSIM bridge
Hi Fabio Estevam, 2023년 3월 14일 (화) 오전 9:31, Fabio Estevam 님이 작성: > Hi Inki, > > On Mon, Mar 6, 2023 at 2:24 AM 대인기/Tizen Platform Lab(SR)/삼성전자 > wrote: > > > Seems some issue Marek found on testing. If fixed then I will try to > pick this > > patch series up. > > Marek has successfully tested v16. > > Could you please apply v16? > I am planning to merge this patch series soon, but I will be proceeding with the pull-request next week. As the DSIM driver is being moved to the bridge folder, I would like to wait for acknowledgment from the bridge maintainers. However, if there are no further comments until next week, I will proceed with the pull-request. Thanks, Inki Dae > Thanks >
Re: [PATCH v15 00/16] drm: Add Samsung MIPI DSIM bridge
Hi Inki, On Mon, Mar 6, 2023 at 2:24 AM 대인기/Tizen Platform Lab(SR)/삼성전자 wrote: > Seems some issue Marek found on testing. If fixed then I will try to pick this > patch series up. Marek has successfully tested v16. Could you please apply v16? Thanks
Re: [PATCH v3 10/10] arm64: dts: qcom: sm6115: Use the correct DSI compatible
On 2023-03-07 14:01:48, Konrad Dybcio wrote:
> Use the non-deprecated, SoC-specific DSI compatible.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Marijn Suijten
> ---
> arch/arm64/boot/dts/qcom/sm6115.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> index 4d6ec815b78b..26e2c7919961 100644
> --- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
> @@ -1218,7 +1218,7 @@ opp-38400 {
> };
>
> mdss_dsi0: dsi@5e94000 {
> - compatible = "qcom,dsi-ctrl-6g-qcm2290";
> + compatible = "qcom,sm6115-dsi-ctrl",
> "qcom,mdss-dsi-ctrl";
This is what the example should look like in qcom,sm6115-mdss.yaml, too.
- Marijn
> reg = <0x0 0x05e94000 0x0 0x400>;
> reg-names = "dsi_ctrl";
>
>
> --
> 2.39.2
>
linux-next: manual merge of the drm-misc tree with Linus' tree
Hi all,
Today's linux-next merge of the drm-misc tree got a conflict in:
drivers/gpu/drm/tiny/cirrus.c
between commit:
7245e629dcaa ("drm/cirrus: NULL-check pipe->plane.state->fb in
cirrus_pipe_update()")
from Linus' tree and commits:
d99c028941b3 ("drm/cirrus: Convert to regular atomic helpers")
03e7ac67e743 ("drm/cirrus: Enable damage clipping on primary plane")
from the drm-misc tree.
I fixed it up (I just used the latter version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
pgpdlKw0f0Xi0.pgp
Description: OpenPGP digital signature
Re: [PATCH v3 09/10] dt-bindings: display/msm: dsi-controller-main: Add SM6115
On 2023-03-08 12:51:03, Rob Herring wrote: > > On Tue, 07 Mar 2023 14:01:47 +0100, Konrad Dybcio wrote: > > Add a compatible for the DSI on SM6115. > > > > Signed-off-by: Konrad Dybcio > > --- > > .../devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 ++ > > .../devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml | 8 > > +++- > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > Acked-by: Rob Herring Shouldn't the examples in qcom,sm6115-mdss.yaml be updated below to reflect the binding changes?
Re: [PATCH v3 08/10] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
On 2023-03-07 14:01:46, Konrad Dybcio wrote:
> The point of the previous cleanup was to disallow "qcom,mdss-dsi-ctrl"
> alone. This however didn't quite work out and the property became
> undocumented instead of deprecated. Fix that.
>
> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible
> strings for every current SoC")
> Signed-off-by: Konrad Dybcio
Yes. From the previous binding (prior to that patch) either
qcom,mdss-dsi-ctrl _or_ qcom,dsi-ctrl-6g-qcm2290 was allowed, not a pair
of both.
Reviewed-by: Marijn Suijten
> ---
> Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index 2494817c1bd6..94f4cdf88c95 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -34,7 +34,7 @@ properties:
>- items:
>- enum:
>- qcom,dsi-ctrl-6g-qcm2290
No comment that it was simply renamed?
> - - const: qcom,mdss-dsi-ctrl
> + - qcom,mdss-dsi-ctrl # This should always come with an
> SoC-specific compatible
> deprecated: true
>
>reg:
>
> --
> 2.39.2
>
Re: [PATCH v3 07/10] drm/msm/dsi: Remove custom DSI config handling
On 2023-03-07 14:01:45, Konrad Dybcio wrote:
> Now that the only user is handled by common code, remove the option to
> specify custom handlers through match data.
>
> This is effectively a revert of commit:
> 5ae15e76271 ("drm/msm/dsi: Allow to specify dsi config as pdata")
Would it also be worth to mention something along these lines in the
previous patch, but for ee1f09678f14 ("drm/msm/dsi: Add support for
qcm2290 dsi controller")?
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi.c | 4 ++--
> drivers/gpu/drm/msm/dsi/dsi_host.c | 4
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 90d43628b22b..e0b911af618d 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -173,10 +173,10 @@ static int dsi_dev_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id dt_match[] = {
> - { .compatible = "qcom,mdss-dsi-ctrl", .data = NULL /* autodetect cfg */
> },
> + { .compatible = "qcom,mdss-dsi-ctrl" },
>
> /* Deprecated, don't use */
> - { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = NULL },
> + { .compatible = "qcom,dsi-ctrl-6g-qcm2290" },
> {}
> };
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 9cfb9e91bfea..961689a255c4 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -214,10 +214,6 @@ static const struct msm_dsi_cfg_handler *dsi_get_config(
> int ret;
> u32 major = 0, minor = 0;
>
> - cfg_hnd = device_get_match_data(dev);
> - if (cfg_hnd)
> - return cfg_hnd;
> -
> ahb_clk = msm_clk_get(msm_host->pdev, "iface");
> if (IS_ERR(ahb_clk)) {
> pr_err("%s: cannot get interface clock\n", __func__);
>
> --
> 2.39.2
>
Re: [PATCH v3 06/10] drm/msm/dsi: Switch the QCM2290-specific compatible to index autodetection
On 2023-03-07 14:01:44, Konrad Dybcio wrote:
> Now that the logic can handle multiple sets of registers, move
> the QCM2290 to the common logic and mark it deprecated. This allows us
> to remove a couple of structs, saving some memory.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/dsi/dsi.c | 4 +++-
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 28 ++--
> 2 files changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index 31fdee2052be..90d43628b22b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -174,7 +174,9 @@ static int dsi_dev_remove(struct platform_device *pdev)
>
> static const struct of_device_id dt_match[] = {
> { .compatible = "qcom,mdss-dsi-ctrl", .data = NULL /* autodetect cfg */
> },
> - { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data =
> _dsi_cfg_handler },
> +
> + /* Deprecated, don't use */
> + { .compatible = "qcom,dsi-ctrl-6g-qcm2290", .data = NULL },
> {}
> };
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index 6d4b2ce4b918..29ccd755cc2e 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -169,7 +169,8 @@ static const struct msm_dsi_config sdm845_dsi_cfg = {
> .bus_clk_names = dsi_v2_4_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names),
> .io_start = {
> - { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 / SC7180 */
> + { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 */
> + { 0x5e94000 }, /* QCM2290 / SM6115 / SM6125 / SM6375 */
> },
> };
>
> @@ -203,25 +204,6 @@ static const struct msm_dsi_config sc7280_dsi_cfg = {
> },
> };
>
> -static const char * const dsi_qcm2290_bus_clk_names[] = {
> - "iface", "bus",
> -};
> -
> -static const struct regulator_bulk_data qcm2290_dsi_cfg_regulators[] = {
> - { .supply = "vdda", .init_load_uA = 21800 },/* 1.2 V */
> -};
These two consts should really have already been deleted as part of
04/10: drm/msm/dsi: dsi_cfg: Deduplicate identical structs.
> -static const struct msm_dsi_config qcm2290_dsi_cfg = {
> - .io_offset = DSI_6G_REG_SHIFT,
> - .regulator_data = qcm2290_dsi_cfg_regulators,
> - .num_regulators = ARRAY_SIZE(qcm2290_dsi_cfg_regulators),
> - .bus_clk_names = dsi_qcm2290_bus_clk_names,
> - .num_bus_clks = ARRAY_SIZE(dsi_qcm2290_bus_clk_names),
> - .io_start = {
> - { 0x5e94000 },
> - },
> -};
> -
> static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = {
> .link_clk_set_rate = dsi_link_clk_set_rate_v2,
> .link_clk_enable = dsi_link_clk_enable_v2,
> @@ -312,9 +294,3 @@ const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32
> major, u32 minor)
>
> return cfg_hnd;
> }
> -
> -/* Non autodetect configs */
> -const struct msm_dsi_cfg_handler qcm2290_dsi_cfg_handler = {
> - .cfg = _dsi_cfg,
> - .ops = _dsi_6g_v2_host_ops,
> -};
And how do you think dsi.c is able to reference this... don't forget to
remove it from dsi_cfg.h in v4. In fact, if you look at how this was
implemented you should also be able to remove #include "dsi_cfg.h" from
dsi.c. A clean revert of that patch would be nice, or just use it as
reference to find the remnants:
https://lore.kernel.org/all/1644853060-1-2-git-send-email-loic.poul...@linaro.org/
- Marijn
Re: [PATCH v3 05/10] drm/msm/dsi: dsi_cfg: Merge SC7180 config into SDM845
On 2023-03-07 14:01:43, Konrad Dybcio wrote:
> The configs are identical, other than the number of *maximum* DSI
> hosts allowed. This isn't an issue, unless somebody deliberately
> tries to access the inexistent host by adding a dt node for it.
>
> Remove the SC7180 struct and point the hw revision match to the
> SDM845's one. On a note, this could have been done back when
> 7180 support was introduced.
Maybe differences were expected, but then sdm845_dsi_cfg is used on many
other revisions.
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 15 ++-
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index d39521850018..6d4b2ce4b918 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -169,7 +169,7 @@ static const struct msm_dsi_config sdm845_dsi_cfg = {
> .bus_clk_names = dsi_v2_4_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names),
> .io_start = {
> - { 0xae94000, 0xae96000 },
> + { 0xae94000, 0xae96000 }, /* SDM845 / SDM670 / SC7180 */
> },
> };
>
> @@ -188,17 +188,6 @@ static const struct msm_dsi_config sm8550_dsi_cfg = {
> },
> };
>
> -static const struct msm_dsi_config sc7180_dsi_cfg = {
> - .io_offset = DSI_6G_REG_SHIFT,
> - .regulator_data = dsi_v2_4_regulators,
> - .num_regulators = ARRAY_SIZE(dsi_v2_4_regulators),
> - .bus_clk_names = dsi_v2_4_clk_names,
> - .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names),
> - .io_start = {
> - { 0xae94000 },
> - },
> -};
> -
> static const struct regulator_bulk_data sc7280_dsi_regulators[] = {
> { .supply = "vdda", .init_load_uA = 8350 }, /* 1.2 V */
> };
> @@ -299,7 +288,7 @@ static const struct msm_dsi_cfg_handler
> dsi_cfg_handlers[] = {
> {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_4_0,
> _dsi_cfg, _dsi_6g_v2_host_ops},
> {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_4_1,
> - _dsi_cfg, _dsi_6g_v2_host_ops},
> + _dsi_cfg, _dsi_6g_v2_host_ops},
> {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_5_0,
> _dsi_cfg, _dsi_6g_v2_host_ops},
> {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_6_0,
>
> --
> 2.39.2
>
Re: [PATCH v3 04/10] drm/msm/dsi: dsi_cfg: Deduplicate identical structs
On 2023-03-07 14:01:42, Konrad Dybcio wrote:
> Some structs were defined multiple times for no apparent reason.
> Deduplicate them.
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Seems a bit inconsistent to name some of these with their DSI host
revision, and keep some named after the SoC. Also in the name of
msm_dsi_config. Regardless:
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 77
> +--
> 1 file changed, 26 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index 6c192963c100..d39521850018 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -47,41 +47,32 @@ static const struct msm_dsi_config
> msm8974_apq8084_dsi_cfg = {
> },
> };
>
> -static const char * const dsi_8916_bus_clk_names[] = {
> +static const char * const dsi_v1_3_1_clk_names[] = {
> "mdp_core", "iface", "bus",
> };
>
> -static const struct regulator_bulk_data msm8916_dsi_regulators[] = {
> +static const struct regulator_bulk_data dsi_v1_3_1_regulators[] = {
> { .supply = "vdda", .init_load_uA = 10 }, /* 1.2 V */
> { .supply = "vddio", .init_load_uA = 10 }, /* 1.8 V */
> };
>
> static const struct msm_dsi_config msm8916_dsi_cfg = {
> .io_offset = DSI_6G_REG_SHIFT,
> - .regulator_data = msm8916_dsi_regulators,
> - .num_regulators = ARRAY_SIZE(msm8916_dsi_regulators),
> - .bus_clk_names = dsi_8916_bus_clk_names,
> - .num_bus_clks = ARRAY_SIZE(dsi_8916_bus_clk_names),
> + .regulator_data = dsi_v1_3_1_regulators,
> + .num_regulators = ARRAY_SIZE(dsi_v1_3_1_regulators),
> + .bus_clk_names = dsi_v1_3_1_clk_names,
> + .num_bus_clks = ARRAY_SIZE(dsi_v1_3_1_clk_names),
> .io_start = {
> { 0x1a98000 },
> },
> };
>
> -static const char * const dsi_8976_bus_clk_names[] = {
> - "mdp_core", "iface", "bus",
> -};
> -
> -static const struct regulator_bulk_data msm8976_dsi_regulators[] = {
> - { .supply = "vdda", .init_load_uA = 10 }, /* 1.2 V */
> - { .supply = "vddio", .init_load_uA = 10 }, /* 1.8 V */
> -};
> -
> static const struct msm_dsi_config msm8976_dsi_cfg = {
> .io_offset = DSI_6G_REG_SHIFT,
> - .regulator_data = msm8976_dsi_regulators,
> - .num_regulators = ARRAY_SIZE(msm8976_dsi_regulators),
> - .bus_clk_names = dsi_8976_bus_clk_names,
> - .num_bus_clks = ARRAY_SIZE(dsi_8976_bus_clk_names),
> + .regulator_data = dsi_v1_3_1_regulators,
> + .num_regulators = ARRAY_SIZE(dsi_v1_3_1_regulators),
> + .bus_clk_names = dsi_v1_3_1_clk_names,
> + .num_bus_clks = ARRAY_SIZE(dsi_v1_3_1_clk_names),
> .io_start = {
> { 0x1a94000, 0x1a96000 },
> },
> @@ -107,10 +98,6 @@ static const struct msm_dsi_config msm8994_dsi_cfg = {
> },
> };
>
> -static const char * const dsi_8996_bus_clk_names[] = {
> - "mdp_core", "iface", "bus", "core_mmss",
> -};
> -
> static const struct regulator_bulk_data msm8996_dsi_regulators[] = {
> { .supply = "vdda", .init_load_uA = 18160 },/* 1.25 V */
> { .supply = "vcca", .init_load_uA = 17000 },/* 0.925 V */
> @@ -121,8 +108,8 @@ static const struct msm_dsi_config msm8996_dsi_cfg = {
> .io_offset = DSI_6G_REG_SHIFT,
> .regulator_data = msm8996_dsi_regulators,
> .num_regulators = ARRAY_SIZE(msm8996_dsi_regulators),
> - .bus_clk_names = dsi_8996_bus_clk_names,
> - .num_bus_clks = ARRAY_SIZE(dsi_8996_bus_clk_names),
> + .bus_clk_names = dsi_6g_bus_clk_names,
> + .num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
> .io_start = {
> { 0x994000, 0x996000 },
> },
> @@ -167,24 +154,20 @@ static const struct msm_dsi_config sdm660_dsi_cfg = {
> },
> };
>
> -static const char * const dsi_sdm845_bus_clk_names[] = {
> +static const char * const dsi_v2_4_clk_names[] = {
> "iface", "bus",
> };
>
> -static const char * const dsi_sc7180_bus_clk_names[] = {
> - "iface", "bus",
> -};
> -
> -static const struct regulator_bulk_data sdm845_dsi_regulators[] = {
> +static const struct regulator_bulk_data dsi_v2_4_regulators[] = {
> { .supply = "vdda", .init_load_uA = 21800 },/* 1.2 V */
> };
>
> static const struct msm_dsi_config sdm845_dsi_cfg = {
> .io_offset = DSI_6G_REG_SHIFT,
> - .regulator_data = sdm845_dsi_regulators,
> - .num_regulators = ARRAY_SIZE(sdm845_dsi_regulators),
> - .bus_clk_names = dsi_sdm845_bus_clk_names,
> - .num_bus_clks = ARRAY_SIZE(dsi_sdm845_bus_clk_names),
> + .regulator_data = dsi_v2_4_regulators,
> + .num_regulators = ARRAY_SIZE(dsi_v2_4_regulators),
> + .bus_clk_names = dsi_v2_4_clk_names,
> + .num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names),
> .io_start = {
> { 0xae94000, 0xae96000 },
> },
> @@ -198,32 +181,24 @@ static const struct
Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Allow for very slow GuC loading
On Fri, 10 Mar 2023 17:01:42 -0800, John Harrison wrote:
>
> >> + for (count = 0; count < 20; count++) {
> >> + ret = wait_for(guc_load_done(uncore, , ), 1000);
> >
> > Isn't 20 secs a bit too long for an in-place wait? I get that if the GuC
> > doesn't load (or fail to) within a few secs the HW is likely toast, but
> > still that seems a bit too long to me. What's the worst case load time
> > ever observed? I suggest reducing the wait to 3 secs as a compromise, if
> > that's bigger than the worst case.
>
> I can drop it to 3 for normal builds and keep 20 for
> CONFIG_DRM_I915_DEBUG_GEM builds. However, that won't actually be long
> enough for all slow situations. We have seen times of at least 11s when the
> GPU is running at minimum frequency. So, for CI runs we definitely want to
> keep the 20s limit. For end users? Is it better to wait for up to 20s or to
> boot in display only fallback mode? And note that this is a timeout only. A
> functional system will still complete in tens of milliseconds.
Just FYI, in this related patch:
https://patchwork.freedesktop.org/series/115003/#rev2
I am holding a mutex across GuC FW load, so very unlikely, but worst case a
thread can get blocked for the duration of the GuC reset/FW load.
Ashutosh
Re: [PATCH v3 03/10] drm/msm/dsi: Fix DSI index detection when version clash occurs
On 2023-03-07 14:01:41, Konrad Dybcio wrote:
> Currently, we allow for MAX_DSI entries in io_start to facilitate for
> MAX_DSI number of DSI hosts at different addresses. The configuration
> is matched against the DSI CTRL hardware revision read back from the
> component. We need a way to resolve situations where multiple SoCs
> with different register maps may use the same version of DSI CTRL. In
> preparation to do so, make msm_dsi_config a 2d array where each entry
> represents a set of configurations adequate for a given SoC.
Note that this code isn't fool-proof against different SoCs sharing the
same DSI host address but for different indices (for example, the
address at variant 0 DSI 0 could be the same as variant 1 DSI 1) and the
matching logic would wrongly return ID 0 instead of 1 for SoC variant 1,
because that's the first matching address it finds.
> This is totally fine to do, as the only differentiating factors
> between same-version-different-SoCs configurations are the number of
> DSI hosts (1 or 2, at least as of today) and the set of base registers.
> The regulator setup is the same, because the DSI hardware is the same,
> regardless of the SoC it was implemented in.
>
> In addition to that, update the matching logic such that it will loop
> over VARIANTS_MAX variants, making sure they are all taken into account.
"in addition to that" makes it sound like you're doing a separate new
thing in this patch, when the match logic must in fact be updated to
make it compatible with the change described above (as in, it doesn't
compile if you don't account for the extra depth in the array).
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Regardless of the above, I don't think it's a problem right now and I
really like the direction this is headed in: miles better than having a
single distinct SoC with a separate way (compatible) of selecting the
host DSI CTRL, rather than the hw revision readback.
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 52
> --
> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 5 +++-
> drivers/gpu/drm/msm/dsi/dsi_host.c | 10
> 3 files changed, 48 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index 4515f52b407a..6c192963c100 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -21,7 +21,9 @@ static const struct msm_dsi_config apq8064_dsi_cfg = {
> .num_regulators = ARRAY_SIZE(apq8064_dsi_regulators),
> .bus_clk_names = dsi_v2_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_v2_bus_clk_names),
> - .io_start = { 0x470, 0x580 },
> + .io_start = {
> + { 0x470, 0x580 },
> + },
> };
>
> static const char * const dsi_6g_bus_clk_names[] = {
> @@ -40,7 +42,9 @@ static const struct msm_dsi_config msm8974_apq8084_dsi_cfg
> = {
> .num_regulators = ARRAY_SIZE(msm8974_apq8084_regulators),
> .bus_clk_names = dsi_6g_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
> - .io_start = { 0xfd922800, 0xfd922b00 },
> + .io_start = {
> + { 0xfd922800, 0xfd922b00 },
> + },
> };
>
> static const char * const dsi_8916_bus_clk_names[] = {
> @@ -58,7 +62,9 @@ static const struct msm_dsi_config msm8916_dsi_cfg = {
> .num_regulators = ARRAY_SIZE(msm8916_dsi_regulators),
> .bus_clk_names = dsi_8916_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_8916_bus_clk_names),
> - .io_start = { 0x1a98000 },
> + .io_start = {
> + { 0x1a98000 },
> + },
> };
>
> static const char * const dsi_8976_bus_clk_names[] = {
> @@ -76,7 +82,9 @@ static const struct msm_dsi_config msm8976_dsi_cfg = {
> .num_regulators = ARRAY_SIZE(msm8976_dsi_regulators),
> .bus_clk_names = dsi_8976_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_8976_bus_clk_names),
> - .io_start = { 0x1a94000, 0x1a96000 },
> + .io_start = {
> + { 0x1a94000, 0x1a96000 },
> + },
> };
>
> static const struct regulator_bulk_data msm8994_dsi_regulators[] = {
> @@ -94,7 +102,9 @@ static const struct msm_dsi_config msm8994_dsi_cfg = {
> .num_regulators = ARRAY_SIZE(msm8994_dsi_regulators),
> .bus_clk_names = dsi_6g_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
> - .io_start = { 0xfd998000, 0xfd9a },
> + .io_start = {
> + { 0xfd998000, 0xfd9a },
> + },
> };
>
> static const char * const dsi_8996_bus_clk_names[] = {
> @@ -113,7 +123,9 @@ static const struct msm_dsi_config msm8996_dsi_cfg = {
> .num_regulators = ARRAY_SIZE(msm8996_dsi_regulators),
> .bus_clk_names = dsi_8996_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_8996_bus_clk_names),
> - .io_start = { 0x994000, 0x996000 },
> + .io_start = {
> + { 0x994000, 0x996000 },
> + },
>
[PATCH v2] drm/i915/guc: Disable PL1 power limit when loading GuC firmware
On dGfx, the PL1 power limit being enabled and set to a low value results
in a low GPU operating freq. It also negates the freq raise operation which
is done before GuC firmware load. As a result GuC firmware load can time
out. Such timeouts were seen in the GL #8062 bug below (where the PL1 power
limit was enabled and set to a low value). Therefore disable the PL1 power
limit when allowed by HW when loading GuC firmware.
v2:
- Take mutex (to disallow writes to power1_max) across GuC reset/fw load
- Add hwm_power_max_restore to error return code path
Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8062
Signed-off-by: Ashutosh Dixit
---
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 10 ++-
drivers/gpu/drm/i915/i915_hwmon.c | 39 +++
drivers/gpu/drm/i915/i915_hwmon.h | 7 +
3 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 4ccb4be4c9cb..15f8e94edc61 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -18,6 +18,7 @@
#include "intel_uc.h"
#include "i915_drv.h"
+#include "i915_hwmon.h"
static const struct intel_uc_ops uc_ops_off;
static const struct intel_uc_ops uc_ops_on;
@@ -460,7 +461,7 @@ static int __uc_init_hw(struct intel_uc *uc)
struct drm_i915_private *i915 = gt->i915;
struct intel_guc *guc = >guc;
struct intel_huc *huc = >huc;
- int ret, attempts;
+ int ret, attempts, pl1en;
GEM_BUG_ON(!intel_uc_supports_guc(uc));
GEM_BUG_ON(!intel_uc_wants_guc(uc));
@@ -491,6 +492,9 @@ static int __uc_init_hw(struct intel_uc *uc)
else
attempts = 1;
+ /* Disable PL1 limit before raising freq */
+ hwm_power_max_disable(gt, );
+
intel_rps_raise_unslice(_to_gt(uc)->rps);
while (attempts--) {
@@ -547,6 +551,8 @@ static int __uc_init_hw(struct intel_uc *uc)
intel_rps_lower_unslice(_to_gt(uc)->rps);
}
+ hwm_power_max_restore(gt, pl1en); /* Restore PL1 limit */
+
guc_info(guc, "submission %s\n",
str_enabled_disabled(intel_uc_uses_guc_submission(uc)));
guc_info(guc, "SLPC %s\n",
str_enabled_disabled(intel_uc_uses_guc_slpc(uc)));
@@ -563,6 +569,8 @@ static int __uc_init_hw(struct intel_uc *uc)
/* Return GT back to RPn */
intel_rps_lower_unslice(_to_gt(uc)->rps);
+ hwm_power_max_restore(gt, pl1en); /* Restore PL1 limit */
+
__uc_sanitize(uc);
if (!ret) {
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c
b/drivers/gpu/drm/i915/i915_hwmon.c
index ee63a8fd88fc..2bbca75ac477 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -444,6 +444,45 @@ hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int
chan, long val)
}
}
+void hwm_power_max_disable(struct intel_gt *gt, u32 *old)
+{
+ struct i915_hwmon *hwmon = gt->i915->hwmon;
+ intel_wakeref_t wakeref;
+ u32 r;
+
+ if (!hwmon || !i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
+ return;
+
+ /* Take mutex to prevent concurrent hwm_power_max_write */
+ mutex_lock(>hwmon_lock);
+
+ with_intel_runtime_pm(hwmon->ddat.uncore->rpm, wakeref)
+ r = intel_uncore_rmw(hwmon->ddat.uncore,
+hwmon->rg.pkg_rapl_limit,
+PKG_PWR_LIM_1_EN, 0);
+
+ *old = !!(r & PKG_PWR_LIM_1_EN);
+
+ /* hwmon_lock mutex is unlocked in hwm_power_max_restore */
+}
+
+void hwm_power_max_restore(struct intel_gt *gt, u32 old)
+{
+ struct i915_hwmon *hwmon = gt->i915->hwmon;
+ intel_wakeref_t wakeref;
+
+ if (!hwmon || !i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit))
+ return;
+
+ with_intel_runtime_pm(hwmon->ddat.uncore->rpm, wakeref)
+ intel_uncore_rmw(hwmon->ddat.uncore,
+hwmon->rg.pkg_rapl_limit,
+PKG_PWR_LIM_1_EN,
+old ? PKG_PWR_LIM_1_EN : 0);
+
+ mutex_unlock(>hwmon_lock);
+}
+
static umode_t
hwm_energy_is_visible(const struct hwm_drvdata *ddat, u32 attr)
{
diff --git a/drivers/gpu/drm/i915/i915_hwmon.h
b/drivers/gpu/drm/i915/i915_hwmon.h
index 7ca9cf2c34c9..0c2db11be2e2 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.h
+++ b/drivers/gpu/drm/i915/i915_hwmon.h
@@ -7,14 +7,21 @@
#ifndef __I915_HWMON_H__
#define __I915_HWMON_H__
+#include
+
struct drm_i915_private;
+struct intel_gt;
#if IS_REACHABLE(CONFIG_HWMON)
void i915_hwmon_register(struct drm_i915_private *i915);
void i915_hwmon_unregister(struct drm_i915_private *i915);
+void hwm_power_max_disable(struct intel_gt *gt, u32 *old);
+void hwm_power_max_restore(struct intel_gt *gt, u32 old);
#else
static inline void i915_hwmon_register(struct drm_i915_private *i915) { };
static inline void
Re: [PATCH drm-next v2 05/16] drm: manager to keep track of GPUs VA mappings
On 3/7/23 23:43, Liam R. Howlett wrote: * Danilo Krummrich [230306 10:46]: On 3/2/23 03:38, Liam R. Howlett wrote: * Danilo Krummrich [230227 08:17]: ... Would this variant be significantly more efficient? Well, what you are doing is walking the tree to see if there's anything there... then re-walking the tree to store it. So, yes, it's much more efficient.. However, writing is heavier. How much of the time is spent walking vs writing depends on the size of the tree, but it's rather easy to do this in a single walk of the tree so why wouldn't you? I will, I was just curious about how much of an impact it has. Also, would this also work while already walking the tree? Yes, to an extent. If you are at the correct location in the tree, you can write to that location. If you are not in the correct location and try to write to the tree then things will go poorly.. In this scenario, we are very much walking the tree and writing to it in two steps. To remove an entry while walking the tree I have a separate function drm_gpuva_iter_remove(). Would I need something similar for inserting entries? I saw that. Your remove function uses the erase operation which is implemented as a walk to that location and a store of a null over the range that is returned. You do not need a function to insert an entry if the maple state is at the correct location, and that doesn't just mean setting mas.index/mas.last to the correct value. There is a node & offset saved in the maple state that needs to be in the correct location. If you store to that node then the node may be replaced, so other iterators that you have may become stale, but the one you used execute the store operation will now point to the new node with the new entry. I already provided this example in a separate mail thread, but it may makes sense to move this to the mailing list: In __drm_gpuva_sm_map() we're iterating a given range of the tree, where the given range is the size of the newly requested mapping. __drm_gpuva_sm_map() invokes a callback for each sub-operation that needs to be taken in order to fulfill this mapping request. In most cases such a callback just creates a drm_gpuva_op object and stores it in a list. However, drivers can also implement the callback, such that they directly execute this operation within the callback. Let's have a look at the following example: 0 a 2 old: |---| (bo_offset=n) 1 b 3 req: |---| (bo_offset=m) 0 a' 1 b 3 new: |-|---| (a.bo_offset=n,b.bo_offset=m) This would result in the following operations. __drm_gpuva_sm_map() finds entry "a" and calls back into the driver suggesting to re-map "a" with the new size. The driver removes entry "a" from the tree and adds "a'" What you have here won't work. The driver will cause your iterators maple state to point to memory that is freed. You will either need to pass through your iterator so that the modifications can occur with that maple state so it remains valid, or you will need to invalidate the iterator on every modification by the driver. I'm sure the first idea you have will be to invalidate the iterator, but that is probably not the way to proceed. Even ignoring the unclear locking of two maple states trying to modify the tree, this is rather inefficient - each invalidation means a re-walk of the tree. You may as well not use an iterator in this case. Depending on how/when the lookups occur, you could still iterate over the tree and let the driver modify the ending of "a", but leave the tree alone and just store b over whatever - but the failure scenarios may cause you grief. If you pass the iterator through, then you can just use it to do your writes and keep iterating as if nothing changed. Passing through the iterater clearly seems to be the way to go. I assume that if the entry to insert isn't at the location of the iterator (as in the following example) we can just keep walking to this location my changing the index of the mas and calling mas_walk()? no. You have to mas_set() to the value and walk from the top of the tree. mas_walk() walks down, not from side to side - well, it does go forward within a node (increasing offset), but if you hit the node limit then you have gotten yourself in trouble. This would also imply that the "outer" tree walk continues after the entry we just inserted, right? I don't understand the "outer" tree walk statement. I think I could have phrased this better. I just mean "my" iterator walking each tree entry rather than an internal tree walk, as it happens in e.g. mas_walk() or mas_find(). 1 a 3 old: |---| (bo_offset=n) 0 b 2 req: |---| (bo_offset=m) 0 b 2 a' 3 new: |---|-| (b.bo_offset=m,a.bo_offset=n+2) Again, after finding "a", we want to remove it and insert "a'" instead. Ah, so
Re: [PATCH v3 02/10] drm/msm/dsi: Get rid of msm_dsi_config::num_dsi
On 2023-03-07 14:01:40, Konrad Dybcio wrote:
> In preparation for supporting multiple sets of possible base registers,
> remove the num_dsi variable. We're comparing the io_start array contents
> with the reg value from the DTS, so it will either match one of the
> expected values or don't match against a zero (which we get from partial
"but won't match against zero" (and drop "either"), that makes this
sentence flow more naturally IMO (otherwise "fail to match against
zero").
> array initialization).
>
> Reviewed-by: Dmitry Baryshkov
> Signed-off-by: Konrad Dybcio
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_cfg.c | 13 -
> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 -
> drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +-
> 3 files changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index 6d21f0b33411..4515f52b407a 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -22,7 +22,6 @@ static const struct msm_dsi_config apq8064_dsi_cfg = {
> .bus_clk_names = dsi_v2_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_v2_bus_clk_names),
> .io_start = { 0x470, 0x580 },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_6g_bus_clk_names[] = {
> @@ -42,7 +41,6 @@ static const struct msm_dsi_config msm8974_apq8084_dsi_cfg
> = {
> .bus_clk_names = dsi_6g_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
> .io_start = { 0xfd922800, 0xfd922b00 },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_8916_bus_clk_names[] = {
> @@ -61,7 +59,6 @@ static const struct msm_dsi_config msm8916_dsi_cfg = {
> .bus_clk_names = dsi_8916_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_8916_bus_clk_names),
> .io_start = { 0x1a98000 },
> - .num_dsi = 1,
> };
>
> static const char * const dsi_8976_bus_clk_names[] = {
> @@ -80,7 +77,6 @@ static const struct msm_dsi_config msm8976_dsi_cfg = {
> .bus_clk_names = dsi_8976_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_8976_bus_clk_names),
> .io_start = { 0x1a94000, 0x1a96000 },
> - .num_dsi = 2,
> };
>
> static const struct regulator_bulk_data msm8994_dsi_regulators[] = {
> @@ -99,7 +95,6 @@ static const struct msm_dsi_config msm8994_dsi_cfg = {
> .bus_clk_names = dsi_6g_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_6g_bus_clk_names),
> .io_start = { 0xfd998000, 0xfd9a },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_8996_bus_clk_names[] = {
> @@ -119,7 +114,6 @@ static const struct msm_dsi_config msm8996_dsi_cfg = {
> .bus_clk_names = dsi_8996_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_8996_bus_clk_names),
> .io_start = { 0x994000, 0x996000 },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_msm8998_bus_clk_names[] = {
> @@ -138,7 +132,6 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
> .bus_clk_names = dsi_msm8998_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_msm8998_bus_clk_names),
> .io_start = { 0xc994000, 0xc996000 },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_sdm660_bus_clk_names[] = {
> @@ -156,7 +149,6 @@ static const struct msm_dsi_config sdm660_dsi_cfg = {
> .bus_clk_names = dsi_sdm660_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_sdm660_bus_clk_names),
> .io_start = { 0xc994000, 0xc996000 },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_sdm845_bus_clk_names[] = {
> @@ -178,7 +170,6 @@ static const struct msm_dsi_config sdm845_dsi_cfg = {
> .bus_clk_names = dsi_sdm845_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_sdm845_bus_clk_names),
> .io_start = { 0xae94000, 0xae96000 },
> - .num_dsi = 2,
> };
>
> static const struct regulator_bulk_data sm8550_dsi_regulators[] = {
> @@ -192,7 +183,6 @@ static const struct msm_dsi_config sm8550_dsi_cfg = {
> .bus_clk_names = dsi_sdm845_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_sdm845_bus_clk_names),
> .io_start = { 0xae94000, 0xae96000 },
> - .num_dsi = 2,
> };
>
> static const struct regulator_bulk_data sc7180_dsi_regulators[] = {
> @@ -206,7 +196,6 @@ static const struct msm_dsi_config sc7180_dsi_cfg = {
> .bus_clk_names = dsi_sc7180_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_sc7180_bus_clk_names),
> .io_start = { 0xae94000 },
> - .num_dsi = 1,
> };
>
> static const char * const dsi_sc7280_bus_clk_names[] = {
> @@ -224,7 +213,6 @@ static const struct msm_dsi_config sc7280_dsi_cfg = {
> .bus_clk_names = dsi_sc7280_bus_clk_names,
> .num_bus_clks = ARRAY_SIZE(dsi_sc7280_bus_clk_names),
> .io_start = { 0xae94000, 0xae96000 },
> - .num_dsi = 2,
> };
>
> static const char * const dsi_qcm2290_bus_clk_names[] = {
> @@ -242,7 +230,6 @@ static const struct msm_dsi_config
Re: [PATCH v3 01/10] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible
On 2023-03-07 14:01:39, Konrad Dybcio wrote:
> The qcom, prefix was missed previously. Fix it.
>
> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible
> strings for every current SoC")
> Acked-by: Rob Herring
> Signed-off-by: Konrad Dybcio
Turns out I got booted from your CC list in b4 and ended up reviewing v2
without being aware of a v3 with previous comments already resolved:
Reviewed-by: Marijn Suijten
> ---
> Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index e75a3efe4dac..2494817c1bd6 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -33,7 +33,7 @@ properties:
>- const: qcom,mdss-dsi-ctrl
>- items:
>- enum:
> - - dsi-ctrl-6g-qcm2290
> + - qcom,dsi-ctrl-6g-qcm2290
>- const: qcom,mdss-dsi-ctrl
> deprecated: true
>
>
> --
> 2.39.2
>
Re: [PATCH v2 1/9] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible
On 2023-02-13 13:10:04, Konrad Dybcio wrote:
> The qcom, prefix was missed previously. Fix it.
>
> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible
> strings for every current SoC")
> Signed-off-by: Konrad Dybcio
Reviewed-by: Marijn Suijten
> ---
> .../devicetree/bindings/display/msm/dsi-controller-main.yaml| 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index e75a3efe4dac..2494817c1bd6 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -33,7 +33,7 @@ properties:
>- const: qcom,mdss-dsi-ctrl
>- items:
>- enum:
> - - dsi-ctrl-6g-qcm2290
> + - qcom,dsi-ctrl-6g-qcm2290
>- const: qcom,mdss-dsi-ctrl
> deprecated: true
>
> --
> 2.39.1
>
[PATCH v2 2/2] drm/xe: Use GT oriented log messages in xe_gt.c
Replace generic log messages with ones dedicated for the GT.
Signed-off-by: Michal Wajdeczko
---
drivers/gpu/drm/xe/xe_gt.c | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
index daa433d0f2f5..a46399c24eb8 100644
--- a/drivers/gpu/drm/xe/xe_gt.c
+++ b/drivers/gpu/drm/xe/xe_gt.c
@@ -20,6 +20,7 @@
#include "xe_gt_clock.h"
#include "xe_gt_mcr.h"
#include "xe_gt_pagefault.h"
+#include "xe_gt_printk.h"
#include "xe_gt_sysfs.h"
#include "xe_gt_tlb_invalidation.h"
#include "xe_gt_topology.h"
@@ -612,15 +613,14 @@ int xe_gt_init(struct xe_gt *gt)
static int do_gt_reset(struct xe_gt *gt)
{
- struct xe_device *xe = gt_to_xe(gt);
int err;
xe_mmio_write32(gt, GEN6_GDRST.reg, GEN11_GRDOM_FULL);
err = xe_mmio_wait32(gt, GEN6_GDRST.reg, 0, GEN11_GRDOM_FULL, 5000,
NULL, false);
if (err)
- drm_err(>drm,
- "GT reset failed to clear GEN11_GRDOM_FULL\n");
+ xe_gt_err(gt, "failed to clear GEN11_GRDOM_FULL (%pe)\n",
+ ERR_PTR(err));
return err;
}
@@ -663,14 +663,13 @@ static int do_gt_restart(struct xe_gt *gt)
static int gt_reset(struct xe_gt *gt)
{
- struct xe_device *xe = gt_to_xe(gt);
int err;
/* We only support GT resets with GuC submission */
if (!xe_device_guc_submission_enabled(gt_to_xe(gt)))
return -ENODEV;
- drm_info(>drm, "GT reset started\n");
+ xe_gt_info(gt, "reset started\n");
xe_gt_sanitize(gt);
@@ -699,7 +698,7 @@ static int gt_reset(struct xe_gt *gt)
err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
XE_WARN_ON(err);
- drm_info(>drm, "GT reset done\n");
+ xe_gt_info(gt, "reset done\n");
return 0;
@@ -708,7 +707,7 @@ static int gt_reset(struct xe_gt *gt)
err_msg:
XE_WARN_ON(xe_uc_start(>uc));
xe_device_mem_access_put(gt_to_xe(gt));
- drm_err(>drm, "GT reset failed, err=%d\n", err);
+ xe_gt_err(gt, "reset failed (%pe)\n", ERR_PTR(err));
return err;
}
@@ -722,15 +721,13 @@ static void gt_reset_worker(struct work_struct *w)
void xe_gt_reset_async(struct xe_gt *gt)
{
- struct xe_device *xe = gt_to_xe(gt);
-
- drm_info(>drm, "Try GT reset\n");
+ xe_gt_info(gt, "trying reset\n");
/* Don't do a reset while one is already in flight */
if (xe_uc_reset_prepare(>uc))
return;
- drm_info(>drm, "Doing GT reset\n");
+ xe_gt_info(gt, "reset queued\n");
queue_work(gt->ordered_wq, >reset.worker);
}
@@ -747,7 +744,6 @@ void xe_gt_suspend_prepare(struct xe_gt *gt)
int xe_gt_suspend(struct xe_gt *gt)
{
- struct xe_device *xe = gt_to_xe(gt);
int err;
/* For now suspend/resume is only allowed with GuC */
@@ -767,7 +763,7 @@ int xe_gt_suspend(struct xe_gt *gt)
xe_device_mem_access_put(gt_to_xe(gt));
XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
- drm_info(>drm, "GT suspended\n");
+ xe_gt_info(gt, "suspended\n");
return 0;
@@ -775,14 +771,13 @@ int xe_gt_suspend(struct xe_gt *gt)
XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
err_msg:
xe_device_mem_access_put(gt_to_xe(gt));
- drm_err(>drm, "GT suspend failed: %d\n", err);
+ xe_gt_err(gt, "suspend failed (%pe)\n", ERR_PTR(err));
return err;
}
int xe_gt_resume(struct xe_gt *gt)
{
- struct xe_device *xe = gt_to_xe(gt);
int err;
xe_device_mem_access_get(gt_to_xe(gt));
@@ -796,7 +791,7 @@ int xe_gt_resume(struct xe_gt *gt)
xe_device_mem_access_put(gt_to_xe(gt));
XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
- drm_info(>drm, "GT resumed\n");
+ xe_gt_info(gt, "resumed\n");
return 0;
@@ -804,7 +799,7 @@ int xe_gt_resume(struct xe_gt *gt)
XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL));
err_msg:
xe_device_mem_access_put(gt_to_xe(gt));
- drm_err(>drm, "GT resume failed: %d\n", err);
+ xe_gt_err(gt, "resume failed (%pe)\n", ERR_PTR(err));
return err;
}
--
2.25.1
[PATCH v2 1/2] drm/xe: Introduce GT oriented log messages
While debugging GT related problems, it's good to know which GT was reporting problems. Introduce helper macros to allow prefix GT logs with GT idetifier. We will use them in upcoming patches. v2: use xe_ prefix (Lucas) Signed-off-by: Michal Wajdeczko Cc: Lucas De Marchi Cc: Jani Nikula --- drivers/gpu/drm/xe/xe_gt_printk.h | 46 +++ 1 file changed, 46 insertions(+) create mode 100644 drivers/gpu/drm/xe/xe_gt_printk.h diff --git a/drivers/gpu/drm/xe/xe_gt_printk.h b/drivers/gpu/drm/xe/xe_gt_printk.h new file mode 100644 index ..f01c8e5653a0 --- /dev/null +++ b/drivers/gpu/drm/xe/xe_gt_printk.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2023 Intel Corporation + */ + +#ifndef _XE_GT_PRINTK_H_ +#define _XE_GT_PRINTK_H_ + +#include + +#include "xe_gt_types.h" + +#define xe_gt_printk(_gt, _level, _fmt, ...) \ + drm_##_level(&(_gt)->xe->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) + +#define xe_gt_err(_gt, _fmt, ...) \ + xe_gt_printk((_gt), err, _fmt, ##__VA_ARGS__) + +#define xe_gt_warn(_gt, _fmt, ...) \ + xe_gt_printk((_gt), warn, _fmt, ##__VA_ARGS__) + +#define xe_gt_notice(_gt, _fmt, ...) \ + xe_gt_printk((_gt), notice, _fmt, ##__VA_ARGS__) + +#define xe_gt_info(_gt, _fmt, ...) \ + xe_gt_printk((_gt), info, _fmt, ##__VA_ARGS__) + +#define xe_gt_dbg(_gt, _fmt, ...) \ + xe_gt_printk((_gt), dbg, _fmt, ##__VA_ARGS__) + +#define xe_gt_err_ratelimited(_gt, _fmt, ...) \ + xe_gt_printk((_gt), err_ratelimited, _fmt, ##__VA_ARGS__) + +#define xe_gt_WARN(_gt, _condition, _fmt, ...) \ + drm_WARN(&(_gt)->xe->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) + +#define xe_gt_WARN_ONCE(_gt, _condition, _fmt, ...) \ + drm_WARN_ONCE(&(_gt)->xe->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) + +#define xe_gt_WARN_ON(_gt, _condition) \ + xe_gt_WARN((_gt), _condition, "%s(%s)", "gt_WARN_ON", __stringify(_condition)) + +#define xe_gt_WARN_ON_ONCE(_gt, _condition) \ + xe_gt_WARN_ONCE((_gt), _condition, "%s(%s)", "gt_WARN_ON_ONCE", __stringify(_condition)) + +#endif -- 2.25.1
Re: [PATCH v2 25/25] drm/pl111: Use GEM DMA fbdev emulation
On Mon, Mar 13, 2023 at 4:51 PM Thomas Zimmermann wrote: > Use the fbdev emulation that is optimized for DMA helpers. Avoids > possible shadow buffering and makes the code simpler. > > Reported-by: Linus Walleij > Link: > https://lore.kernel.org/dri-devel/cacrpkdawsqsnqkjkssosw3hmmhynxfuywxkdszptc-a_uza...@mail.gmail.com/ > Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v2 24/25] drm/mcde: Use GEM DMA fbdev emulation
On Mon, Mar 13, 2023 at 4:51 PM Thomas Zimmermann wrote: > Use the fbdev emulation that is optimized for DMA helpers. Avoids > possible shadow buffering and makes the code simpler. > > Reported-by: Linus Walleij > Link: > https://lore.kernel.org/dri-devel/cacrpkdawsqsnqkjkssosw3hmmhynxfuywxkdszptc-a_uza...@mail.gmail.com/ > Signed-off-by: Thomas Zimmermann Reviewed-by: Linus Walleij Tested-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v2 00/25] drm/dma-helper: Add dedicated fbdev emulation
On Mon, Mar 13, 2023 at 4:51 PM Thomas Zimmermann wrote: > Tested with fbcon and IGT on vc4. Also tested on the U8500 MCDE on Samsung GT-I8190 (Golden) successfully. Yours, Linus Walleij
[PATCH] drm/i915/huc: Cancel HuC delayed load timer on reset.
In the rare case where we do a full GT reset after starting the HuC
load and before it completes (which basically boils down to i915 hanging
during init), we need to cancel the delayed load fence, as it will be
re-initialized in the post-reset recovery.
Fixes: 27536e03271d ("drm/i915/huc: track delayed HuC load with a fence")
Signed-off-by: Daniele Ceraolo Spurio
Cc: Alan Previn
---
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 7 +++
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 7 +--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 72884e21470b..aefdaa62da99 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -241,6 +241,13 @@ static void delayed_huc_load_fini(struct intel_huc *huc)
i915_sw_fence_fini(>delayed_load.fence);
}
+int intel_huc_sanitize(struct intel_huc *huc)
+{
+ delayed_huc_load_complete(huc);
+ intel_uc_fw_sanitize(>fw);
+ return 0;
+}
+
static bool vcs_supported(struct intel_gt *gt)
{
intel_engine_mask_t mask = gt->info.engine_mask;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 52db03620c60..db555b3c1f56 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -41,6 +41,7 @@ struct intel_huc {
} delayed_load;
};
+int intel_huc_sanitize(struct intel_huc *huc);
void intel_huc_init_early(struct intel_huc *huc);
int intel_huc_init(struct intel_huc *huc);
void intel_huc_fini(struct intel_huc *huc);
@@ -54,12 +55,6 @@ bool intel_huc_is_authenticated(struct intel_huc *huc);
void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus);
void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus);
-static inline int intel_huc_sanitize(struct intel_huc *huc)
-{
- intel_uc_fw_sanitize(>fw);
- return 0;
-}
-
static inline bool intel_huc_is_supported(struct intel_huc *huc)
{
return intel_uc_fw_is_supported(>fw);
--
2.37.3
Re: [PATCH RFC 11/18] drm/scheduler: Clean up jobs when the scheduler is torn down
On Fri, 2023-03-10 at 18:58 +0900, Asahi Lina wrote: > On 10/03/2023 04.59, Faith Ekstrand wrote: > > On Thu, 2023-03-09 at 18:43 +0900, Asahi Lina wrote: > > > On 09/03/2023 17.42, Christian König wrote: > > > > Long story short: Don't do this! This is what the Windows > > > > drivers > > > > have > > > > been doing and it creates tons of problems. > > > > Yeah, we tried to do a bit of that in the GL days. It was a bad > > idea. > > I think I should clarify: I was proposing re-queueing innocent jobs > from > innocent queues/VMs that were impacted by a fault. The reason is that > we > may be able to tweak firmware state to force it to do that safely, > during the firmware recovery cycle, such that an aborted job restarts > and then subsequent jobs/commands continue as normal. We can't leave > it > to userspace because if we do nothing, the affected job ends up > incomplete but then everything after it that is already queued still > runs, and that is definitely a recipe for a bigger mess if userspace > wants to seamlessly recover. The firmware recovery cycle is a > "stop-the-world" situation for the GPU (the firmware literally > busy-loops waiting for the driver to set a continue flag in > memory...), > so that's the only real chance that the driver gets to make decisions > about what is going to happen next. Ok, that makes sense. Yes, if you have other jobs on other queues and are able to recover everything that isn't in the faulting VM, that's a good thing. I wasn't sure how hang/fault recovery worked on AGX. In tat case, I don't think there's a dma_fence problem. As long as you keep recovering and killing off any faulting contexts, eventually the good contexts should make progress and those fences should signal. Of course, the firmware recovery cycle may be complex and need (or at least appear to) memory allocation or similar and that's where everything gets hairy. Hopefully, though, if you've already got the resources from the old context, you can re-use them after a bit of clean-up work and still get deterministic and reliable recovery cycles. > Of course, that only works if individual possibly concurrently > running > commands are idempotent, but I think a lot of typical GPU work is? No, that's not a valid assumption. For a single 3D render pass which doesn't do any image or SSBO access, it may be possible to re-run it. However, that won't be true of compute work and isn't necessarily true of back-to-back passes. Lots of modern apps do temporal stuff where one frame depends on the previous and a re-run might screw that up. Also, with Vulkan's memory aliasing, it's hard to tell just from which resources are accessed whether or not a command buffer leaves its input memory undamaged. > (E.g. > any render pass without side effects other than the render targets > and > where the background shader does no loads, or even render passes that > do > loads but where all draws are opaque, which are all things the > current > Gallium driver is intimately familiar with since Crazy Tiler > Optimizations™ need that info to be provided anyway). So I was > wondering > whether it'd make sense to have such an idempotency/restartable flag > on > job submission, and then the driver would do its best to recover and > rerun it if it gets killed by an unrelated concurrent bad job. > > Then again this all depends on an investigation into what we *can* do > during firmware recovery that hasn't happened at all yet. It might be > that it isn't safe to do anything really, or that doing things > depends > on touching even deeper firmware state structs that we treat as > opaque > right now and we really don't want to have to touch... > > But maybe none of this is worth it in practice, it just sounded like > it > could be useful maybe? Maybe? It's not clear to me that such a flag would be useful or even practical to provide from the Mesa side. Ideally, you'd be able to figure out when a fault happens, what VM it happened in and exactly what work was in-flight when it happened and only kill the one guilty VM. However, it sounds like your understanding of the firmware is currently rough enough that doing so may not be practical. In that case, the best thing to do is to kill any VMs which were on the GPU at the time and hope the individual apps are able to recover. > Now that I look at it, we have a lovely "what is this flag doing > anyway" > bit already passed from Mesa through to the firmware we called > ASAHI_RENDER_SET_WHEN_RELOADING_Z_OR_S which, now that I look at it, > is > actually getting set when any attachment (any color, Z, S) is not > being > cleared for that pass (so it's loaded). That could very well be an > "is > not idempotent" flag... and maybe that means the firmware does this > for > us already? Sounds like something to test... I might have some > 16Kx16K > GLmark runs to do concurrent with an evil faulting job now ^^ (and > then > that also means we need to set it when shaders have side
Re: [Intel-xe] [PATCH 1/2] drm/xe: Introduce GT oriented log messages
On Mon, 13 Mar 2023, Lucas De Marchi wrote: > +dri-devel > > On Mon, Mar 13, 2023 at 09:03:56AM +0100, Michal Wajdeczko wrote: >>While debugging GT related problems, it's good to know which GT was >>reporting problems. Introduce helper macros to allow prefix GT logs >>with GT idetifier. We will use them in upcoming patches. >> >>Signed-off-by: Michal Wajdeczko >>--- >> drivers/gpu/drm/xe/xe_gt_printk.h | 45 +++ >> 1 file changed, 45 insertions(+) >> create mode 100644 drivers/gpu/drm/xe/xe_gt_printk.h >> >>diff --git a/drivers/gpu/drm/xe/xe_gt_printk.h >>b/drivers/gpu/drm/xe/xe_gt_printk.h >>new file mode 100644 >>index ..b12a92024126 >>--- /dev/null >>+++ b/drivers/gpu/drm/xe/xe_gt_printk.h >>@@ -0,0 +1,45 @@ >>+/* SPDX-License-Identifier: MIT */ >>+/* >>+ * Copyright © 2023 Intel Corporation >>+ */ >>+ >>+#ifndef _XE_GT_PRINTK_H_ >>+#define _XE_GT_PRINTK_H_ >>+ >>+#include > > missing blank line > >>+#include "xe_gt_types.h" >>+ >>+#define gt_printk(_gt, _level, _fmt, ...) \ > > any exposed interface in headers should have xe_ > prefix. > > I myself find it odd to have these macros that are wrappers over > wrappers and create a silo with xe-speficic debugging macros. > The DRM ones at least are shared across drivers. The pr_dbg(), just > reusing a local define is nice as it's still the same call used > everywhere... but it wouldn't work very well here as we need the extra > parm. My biggest problem with this in i915 always was where to draw the line for adding new macros. Add them for something, and you've set the example that's okay to do. And now we have gt_dbg, huc_dbg, guc_dbg, and all the variants. Many of them single-use or unused, but added for completeness. I display we could have drm subsystem wide variants for drm_crtc, drm_connector, drm_encoder, but we've all just opted not to. It's more verbose at the call sites, but it's obvious everywhere, to everyone. I agree with Lucas, I think the wrappers on wrappers on wrappers on wrappers is counter productive. Yes, that's where we are in i915: guc/huc -> gt -> drm -> dev -> printk > I won't oppose them though since a lot of people seem to like the > approach to help their debug and i915 went through similar discussion. Yeah, well, that's what I said in i915 as well, and it wasn't in my domain, really. And ditto here. But I'd totally block this direction in i915 display code. BR, Jani. > > Lucas De Marchi > >>+ drm_##_level(&(_gt)->xe->drm, "GT%u: " _fmt, (_gt)->info.id, >>##__VA_ARGS__) >>+ >>+#define gt_err(_gt, _fmt, ...) \ >>+ gt_printk((_gt), err, _fmt, ##__VA_ARGS__) >>+ >>+#define gt_warn(_gt, _fmt, ...) \ >>+ gt_printk((_gt), warn, _fmt, ##__VA_ARGS__) >>+ >>+#define gt_notice(_gt, _fmt, ...) \ >>+ gt_printk((_gt), notice, _fmt, ##__VA_ARGS__) >>+ >>+#define gt_info(_gt, _fmt, ...) \ >>+ gt_printk((_gt), info, _fmt, ##__VA_ARGS__) >>+ >>+#define gt_dbg(_gt, _fmt, ...) \ >>+ gt_printk((_gt), dbg, _fmt, ##__VA_ARGS__) >>+ >>+#define gt_err_ratelimited(_gt, _fmt, ...) \ >>+ gt_printk((_gt), err_ratelimited, _fmt, ##__VA_ARGS__) >>+ >>+#define gt_WARN(_gt, _condition, _fmt, ...) \ >>+ drm_WARN(&(_gt)->xe->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, >>##__VA_ARGS__) >>+ >>+#define gt_WARN_ONCE(_gt, _condition, _fmt, ...) \ >>+ drm_WARN_ONCE(&(_gt)->xe->drm, _condition, "GT%u: " _fmt, >>(_gt)->info.id, ##__VA_ARGS__) >>+ >>+#define gt_WARN_ON(_gt, _condition) \ >>+ gt_WARN((_gt), _condition, "%s(%s)", "gt_WARN_ON", >>__stringify(_condition)) >>+ >>+#define gt_WARN_ON_ONCE(_gt, _condition) \ >>+ gt_WARN_ONCE((_gt), _condition, "%s(%s)", "gt_WARN_ON_ONCE", >>__stringify(_condition)) >>+ >>+#endif >>-- >>2.25.1 >> -- Jani Nikula, Intel Open Source Graphics Center
Re: [Linaro-mm-sig] Re: [PATCH 2/2] drm/msm: Embed the hw_fence in msm_gem_submit
Am 13.03.23 um 17:43 schrieb Rob Clark:
On Mon, Mar 13, 2023 at 9:15 AM Christian König
wrote:
Am 13.03.23 um 15:45 schrieb Rob Clark:
On Mon, Mar 13, 2023 at 12:19 AM Christian König
wrote:
Am 11.03.23 um 18:35 schrieb Rob Clark:
From: Rob Clark
Avoid allocating memory in job_run() by embedding the fence in the
submit object. Since msm gpu fences are always 1:1 with msm_gem_submit
we can just use the fence's refcnt to track the submit. And since we
can get the fence ctx from the submit we can just drop the msm_fence
struct altogether. This uses the new dma_fence_init_noref() to deal
with the fact that the fence's refcnt is initialized when the submit is
created, long before job_run().
Well this is a very very bad idea, we made the same mistake with amdgpu
as well.
It's true that you should not have any memory allocation in your run_job
callback, but you could also just allocate the hw fence during job
creation and initializing it later on.
I've suggested to embed the fence into the job for amdgpu because some
people insisted of re-submitting jobs during timeout and GPU reset. This
turned into a nightmare with tons of bug fixes on top of bug fixes on
top of bug fixes because it messes up the job and fence lifetime as
defined by the DRM scheduler and DMA-buf framework.
Luben is currently working on cleaning all this up.
This actually shouldn't be a problem with msm, as the fence doesn't
change if there is a gpu reset. We simply signal the fence for the
offending job, reset the GPU, and re-play the remaining in-flight jobs
(ie. things that already had their job_run() called) with the original
fences. (We don't use gpu sched's reset/timeout handling.. when I
migrated to gpu sched I kept our existing hangcheck/recovery
mechanism.)
That sounds much saner than what we did.
So you basically need the dma_fence reference counting separate to
initializing the other dma_fence fields?
yeah, that was the idea
What would happen if a dma_fence which is not completely initialized
gets freed? E.g. because of an error?
hmm, yes, this would be a problem since ops->release is not set yet..
and I'm relying on that to free the submit
Would it be to much to just keep the handling as it is today and only
allocate the dma_fence without initializing it? If necessary we could
easily add a dma_fence_is_initialized() function which checks the
fence_ops for NULL.
Yeah, that would also be possible
I guess we could split creation of the fence (initializing ops,
refcount) and "arming" it later when the seqno is known? But maybe
that is going to too many lengths to avoid a separate allocation..
I would really like to avoid that. It give people the opportunity once
more to do multiple "arm" operations on the same fence, and that was a
really bad idea for us.
So yeah if that's just to avoid the extra allocation it's probably not
worth it.
Christian.
BR,
-R
Thanks,
Christian.
BR,
-R
Regards,
Christian.
Signed-off-by: Rob Clark
---
Note that this applies on top of https://patchwork.freedesktop.org/series/93035/
out of convenience for myself, but I can re-work it to go before
depending on the order that things land.
drivers/gpu/drm/msm/msm_fence.c | 45 +++-
drivers/gpu/drm/msm/msm_fence.h | 2 +-
drivers/gpu/drm/msm/msm_gem.h| 10 +++
drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++---
drivers/gpu/drm/msm/msm_gpu.c| 4 +--
drivers/gpu/drm/msm/msm_ringbuffer.c | 4 +--
6 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 51b461f32103..51f9f1f0cb66 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -103,14 +103,9 @@ void msm_update_fence(struct msm_fence_context *fctx,
uint32_t fence)
spin_unlock_irqrestore(>spinlock, flags);
}
-struct msm_fence {
- struct dma_fence base;
- struct msm_fence_context *fctx;
-};
-
-static inline struct msm_fence *to_msm_fence(struct dma_fence *fence)
+static inline struct msm_gem_submit *fence_to_submit(struct dma_fence *fence)
{
- return container_of(fence, struct msm_fence, base);
+ return container_of(fence, struct msm_gem_submit, hw_fence);
}
static const char *msm_fence_get_driver_name(struct dma_fence *fence)
@@ -120,20 +115,20 @@ static const char *msm_fence_get_driver_name(struct
dma_fence *fence)
static const char *msm_fence_get_timeline_name(struct dma_fence *fence)
{
- struct msm_fence *f = to_msm_fence(fence);
- return f->fctx->name;
+ struct msm_gem_submit *submit = fence_to_submit(fence);
+ return submit->ring->fctx->name;
}
static bool msm_fence_signaled(struct dma_fence *fence)
{
- struct msm_fence *f = to_msm_fence(fence);
- return msm_fence_completed(f->fctx, f->base.seqno);
+ struct msm_gem_submit *submit = fence_to_submit(fence);
+ return
Re: [PATCH v5 2/2] drm/panel: Add driver for Novatek NT36523
On Mon, Mar 13, 2023 at 09:06:50AM +0100, Neil Armstrong wrote:
> On 11/03/2023 13:46, Jianhua Lu wrote:
> > On Sat, Mar 11, 2023 at 01:38:52PM +0100, Konrad Dybcio wrote:
> > >
> > >
> > > On 11.03.2023 13:32, Jianhua Lu wrote:
> > > > Add a driver for panels using the Novatek NT36523 display driver IC.
> > > >
> > > > Signed-off-by: Jianhua Lu
> > > > ---
> > > [...]
> > >
> > > > +
> > > > +static int nt36523_get_modes(struct drm_panel *panel,
> > > > + struct drm_connector *connector)
> > > > +{
> > > > + struct panel_info *pinfo = to_panel_info(panel);
> > > > + int i;
> > > > +
> > > > + for (i = 0; i < pinfo->desc->num_modes; i++) {
> > > > + const struct drm_display_mode *m =
> > > > >desc->modes[i];
> > > > + struct drm_display_mode *mode;
> > > > +
> > > > + mode = drm_mode_duplicate(connector->dev, m);
> > > > + if (!mode) {
> > > > + dev_err(panel->dev, "failed to add mode
> > > > %ux%u@%u\n",
> > > > + m->hdisplay, m->vdisplay,
> > > > drm_mode_vrefresh(m));
> > > > + return -ENOMEM;
> > > > + }
> > > > +
> > > > + mode->type = DRM_MODE_TYPE_DRIVER;
> > > > + if (pinfo->desc->num_modes == 1)
> > > > + mode->type |= DRM_MODE_TYPE_PREFERRED;
> > > That's not quite correct, as that means "if you have more than one
> > > defined panel mode (say 60Hz and 120 Hz), there will be no preferred one".
> > This piece of code I see in the other panels, so I'm not sure if it is
> > correct.
Jianhua is correct that the same code exists in several places,
and from a quick browse I consider all the cases bogus.
It would be fine if someone volunteered to fix all the panels so we
avoid this bug to creep into more panel drivers.
Sam
[PATCH 30/36] drm/i915/huc: use const struct bus_type pointers
The struct bus_type pointers in the functions
intel_huc_register_gsc_notifier() and
intel_huc_unregister_gsc_notifier() should be a const pointer, as the
structure is not modified anywhere in the functions, and the pointer
they are passed will be a const * in the near future.
Cc: Jani Nikula
Cc: Joonas Lahtinen
Cc: Rodrigo Vivi
Cc: Tvrtko Ursulin
Cc: David Airlie
Cc: Daniel Vetter
Cc: Daniele Ceraolo Spurio
Cc: Alan Previn
Cc: John Harrison
Cc: Greg Kroah-Hartman
Cc: Tony Ye
Cc: Vitaly Lubart
Cc: intel-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Greg Kroah-Hartman
---
Note, this is a patch that is a prepatory cleanup as part of a larger
series of patches that is working on resolving some old driver core
design mistakes. It will build and apply cleanly on top of 6.3-rc2 on
its own, but I'd prefer if I could take it through my driver-core tree
so that the driver core changes can be taken through there for 6.4-rc1.
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 4 ++--
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 410905da8e97..8b453bd7c953 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -183,7 +183,7 @@ static int gsc_notifier(struct notifier_block *nb, unsigned
long action, void *d
return 0;
}
-void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus)
+void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus)
{
int ret;
@@ -200,7 +200,7 @@ void intel_huc_register_gsc_notifier(struct intel_huc *huc,
struct bus_type *bus
}
}
-void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus)
+void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus)
{
if (!huc->delayed_load.nb.notifier_call)
return;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
index 52db03620c60..05d4832f8461 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h
@@ -51,8 +51,8 @@ int intel_huc_check_status(struct intel_huc *huc);
void intel_huc_update_auth_status(struct intel_huc *huc);
bool intel_huc_is_authenticated(struct intel_huc *huc);
-void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus);
-void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type
*bus);
+void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus);
+void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct
bus_type *bus);
static inline int intel_huc_sanitize(struct intel_huc *huc)
{
--
2.39.2
Re: [PATCH RFC 03/18] rust: drm: file: Add File abstraction
On Fri, 2023-03-10 at 07:16 +0900, Asahi Lina wrote:
> On 10/03/2023 06.16, Faith Ekstrand wrote:
> > On Tue, 2023-03-07 at 23:25 +0900, Asahi Lina wrote:
> > > A DRM File is the DRM counterpart to a kernel file structure,
> > > representing an open DRM file descriptor. Add a Rust abstraction
> > > to
> > > allow drivers to implement their own File types that implement
> > > the
> > > DriverFile trait.
> > >
> > > Signed-off-by: Asahi Lina
> > > ---
> > > rust/bindings/bindings_helper.h | 1 +
> > > rust/kernel/drm/drv.rs | 7 ++-
> > > rust/kernel/drm/file.rs | 113
> > >
> > > rust/kernel/drm/mod.rs | 1 +
> > > 4 files changed, 120 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/rust/bindings/bindings_helper.h
> > > b/rust/bindings/bindings_helper.h
> > > index 2a999138c4ae..7d7828faf89c 100644
> > > --- a/rust/bindings/bindings_helper.h
> > > +++ b/rust/bindings/bindings_helper.h
> > > @@ -8,6 +8,7 @@
> > >
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > diff --git a/rust/kernel/drm/drv.rs b/rust/kernel/drm/drv.rs
> > > index 29a465515dc9..1dcb651e1417 100644
> > > --- a/rust/kernel/drm/drv.rs
> > > +++ b/rust/kernel/drm/drv.rs
> > > @@ -144,6 +144,9 @@ pub trait Driver {
> > > /// Should be either `drm::gem::Object` or
> > > `drm::gem::shmem::Object`.
> > > type Object: AllocImpl;
> > >
> > > + /// The type used to represent a DRM File (client)
> > > + type File: drm::file::DriverFile;
> > > +
> > > /// Driver metadata
> > > const INFO: DriverInfo;
> > >
> > > @@ -213,8 +216,8 @@ macro_rules! drm_device_register {
> > > impl Registration {
> > > const VTABLE: bindings::drm_driver = drm_legacy_fields! {
> > > load: None,
> > > - open: None, // TODO: File abstraction
> > > - postclose: None, // TODO: File abstraction
> > > + open: Some(drm::file::open_callback::),
> > > + postclose:
> > > Some(drm::file::postclose_callback::),
> > > lastclose: None,
> > > unload: None,
> > > release: None,
> > > diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
> > > new file mode 100644
> > > index ..48751e93c38a
> > > --- /dev/null
> > > +++ b/rust/kernel/drm/file.rs
> > > @@ -0,0 +1,113 @@
> > > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > > +
> > > +//! DRM File objects.
> > > +//!
> > > +//! C header:
> > > [`include/linux/drm/drm_file.h`](../../../../include/linux/drm/dr
> > > m_fi
> > > le.h)
> > > +
> > > +use crate::{bindings, drm, error::Result};
> > > +use alloc::boxed::Box;
> > > +use core::marker::PhantomData;
> > > +use core::ops::Deref;
> > > +
> > > +/// Trait that must be implemented by DRM drivers to represent a
> > > DRM
> > > File (a client instance).
> > > +pub trait DriverFile {
> > > + /// The parent `Driver` implementation for this
> > > `DriverFile`.
> > > + type Driver: drm::drv::Driver;
> > > +
> > > + /// Open a new file (called when a client opens the DRM
> > > device).
> > > + fn open(device: ::device::Device) ->
> > > Result>;
> > > +}
> > > +
> > > +/// An open DRM File.
> > > +///
> > > +/// # Invariants
> > > +/// `raw` is a valid pointer to a `drm_file` struct.
> > > +#[repr(transparent)]
> > > +pub struct File {
> > > + raw: *mut bindings::drm_file,
> > > + _p: PhantomData,
> > > +}
> > > +
> > > +pub(super) unsafe extern "C" fn open_callback(
> > > + raw_dev: *mut bindings::drm_device,
> > > + raw_file: *mut bindings::drm_file,
> > > +) -> core::ffi::c_int {
> > > + let drm = core::mem::ManuallyDrop::new(unsafe {
> > > drm::device::Device::from_raw(raw_dev) });
> >
> > Maybe you can help educate me a bit here... This feels like a
> > really
> > sketchy pattern. We're creating a Device from a pointer, an
> > operation
> > which inherently consumes a reference but then marking it
> > ManuallyDrop
> > so drm_device_put() never gets called. It took me a while but I
> > think
> > I figured out what you're trying to do: Make it so all the Rust
> > stuff
> > works with Device, not drm_device but it still feels really wrong.
> > It
> > works, it just feels like there's a lot of unsafe abstraction
> > juggling
> > happening here and I expect this operation is going to be pretty
> > common
> > in the Rust abstraction layer.
>
> So I think this is going to be a pretty common pattern in this kind
> of
> abstraction. The problem is that, of course, in C there is no
> distinction between an owned reference and a borrowed one. Here we
> have
> a borrowed reference to a struct drm_device, and we need to turn it
> into
> a (which is the Rust equivalent type). But for to
> exist
> we need a Device to exist in the first place, and Device normally
> implies ownership of the underlying drm_device.
Thanks! Putting it in terms of borrow really helps clear up the
difference.
> We could just acquire a
Re: [PATCH v3 3/8] accel/qaic: Add MHI controller
On 3/13/2023 7:39 AM, Jacek Lawrynowicz wrote:
Hi,
On 06.03.2023 22:33, Jeffrey Hugo wrote:
An AIC100 device contains a MHI interface with a number of different
channels for controlling different aspects of the device. The MHI
controller works with the MHI bus to enable and drive that interface.
AIC100 uses the BHI protocol in PBL to load SBL. The MHI controller
expects the SBL to be located at /lib/firmware/qcom/aic100/sbl.bin and
expects the MHI bus to manage the process of loading and sending SBL to
the device.
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/mhi_controller.c | 563
drivers/accel/qaic/mhi_controller.h | 16 +
2 files changed, 579 insertions(+)
create mode 100644 drivers/accel/qaic/mhi_controller.c
create mode 100644 drivers/accel/qaic/mhi_controller.h
diff --git a/drivers/accel/qaic/mhi_controller.c
b/drivers/accel/qaic/mhi_controller.c
new file mode 100644
index 000..f16dbb7
--- /dev/null
+++ b/drivers/accel/qaic/mhi_controller.c
@@ -0,0 +1,563 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2019-2021, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2021-2023 Qualcomm Innovation Center, Inc. All rights
reserved. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "mhi_controller.h"
+#include "qaic.h"
+
+#define MAX_RESET_TIME_SEC 25
+
+static unsigned int mhi_timeout = 2000; /* 2 sec default */
+module_param(mhi_timeout, uint, 0600);
Consider documenting the param with MODULE_PARM_DESC() and adding _ms postfix to
indicate that time units it is using.
Will do.
+
+static struct mhi_channel_config aic100_channels[] = {
+ {
+ .name = "QAIC_LOOPBACK",
+ .num = 0,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .auto_queue = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_LOOPBACK",
+ .num = 1,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .auto_queue = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_SAHARA",
+ .num = 2,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_SBL,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .auto_queue = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_SAHARA",
+ .num = 3,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_SBL,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .auto_queue = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_DIAG",
+ .num = 4,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .auto_queue = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_DIAG",
+ .num = 5,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+
Re: [PATCH v3 2/8] accel/qaic: Add uapi and core driver file
On 3/13/2023 7:21 AM, Jacek Lawrynowicz wrote:
Hi,
On 06.03.2023 22:33, Jeffrey Hugo wrote:
Add the QAIC driver uapi file and core driver file that binds to the PCIe
device. The core driver file also creates the accel device and manages
all the interconnections between the different parts of the driver.
The driver can be built as a module. If so, it will be called "qaic.ko".
Signed-off-by: Jeffrey Hugo
Reviewed-by: Carl Vanderlip
Reviewed-by: Pranjal Ramajor Asha Kanojiya
---
drivers/accel/qaic/qaic.h | 282 ++
drivers/accel/qaic/qaic_drv.c | 648 ++
include/uapi/drm/qaic_accel.h | 397 ++
3 files changed, 1327 insertions(+)
create mode 100644 drivers/accel/qaic/qaic.h
create mode 100644 drivers/accel/qaic/qaic_drv.c
create mode 100644 include/uapi/drm/qaic_accel.h
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
new file mode 100644
index 000..996a68f
--- /dev/null
+++ b/drivers/accel/qaic/qaic.h
@@ -0,0 +1,282 @@
+/* SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2021-2023 Qualcomm Innovation Center, Inc. All rights
reserved.
+ */
+
+#ifndef _QAIC_H_
+#define _QAIC_H_
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define QAIC_DBC_BASE SZ_128K
+#define QAIC_DBC_SIZE SZ_4K
+
+#define QAIC_NO_PARTITION -1
+
+#define QAIC_DBC_OFF(i)((i) * QAIC_DBC_SIZE + QAIC_DBC_BASE)
+
+#define to_qaic_bo(obj) container_of(obj, struct qaic_bo, base)
+
+extern bool poll_datapath;
+
+struct qaic_user {
+ /* Uniquely identifies this user for the device */
+ int handle;
+ struct kref ref_count;
+ /* Char device opened by this user */
+ struct qaic_drm_device *qddev;
+ /* Node in list of users that opened this drm device */
+ struct list_headnode;
+ /* SRCU used to synchronize this user during cleanup */
+ struct srcu_struct qddev_lock;
+ atomic_tchunk_id;
+};
+
+struct dma_bridge_chan {
+ /* Pointer to device strcut maintained by driver */
+ struct qaic_device *qdev;
+ /* ID of this DMA bridge channel(DBC) */
+ unsigned intid;
+ /* Synchronizes access to xfer_list */
+ spinlock_t xfer_lock;
+ /* Base address of request queue */
+ void*req_q_base;
+ /* Base address of response queue */
+ void*rsp_q_base;
+ /*
+* Base bus address of request queue. Response queue bus address can be
+* calculated by adding request queue size to this variable
+*/
+ dma_addr_t dma_addr;
+ /* Total size of request and response queue in byte */
+ u32 total_size;
+ /* Capacity of request/response queue */
+ u32 nelem;
+ /* The user that opened this DBC */
+ struct qaic_user*usr;
+ /*
+* Request ID of next memory handle that goes in request queue. One
+* memory handle can enqueue more than one request elements, all
+* this requests that belong to same memory handle have same request ID
+*/
+ u16 next_req_id;
+ /* true: DBC is in use; false: DBC not in use */
+ boolin_use;
+ /*
+* Base address of device registers. Used to read/write request and
+* response queue's head and tail pointer of this DBC.
+*/
+ void __iomem*dbc_base;
+ /* Head of list where each node is a memory handle queued in request
queue */
+ struct list_headxfer_list;
+ /* Synchronizes DBC readers during cleanup */
+ struct srcu_struct ch_lock;
+ /*
+* When this DBC is released, any thread waiting on this wait queue is
+* woken up
+*/
+ wait_queue_head_t dbc_release;
+ /* Head of list where each node is a bo associated with this DBC */
+ struct list_headbo_lists;
+ /* The irq line for this DBC. Used for polling */
+ unsigned intirq;
+ /* Polling work item to simulate interrupts */
+ struct work_struct poll_work;
+};
+
+struct qaic_device {
+ /* Pointer to base PCI device struct of our physical device */
+ struct pci_dev *pdev;
+ /* Req. ID of request that will be queued next in MHI control device */
+ u32 next_seq_num;
+ /* Base address of bar 0 */
+ void __iomem*bar_0;
+ /* Base address of bar 2 */
+ void __iomem*bar_2;
+ /* Controller structure for MHI devices */
+ struct mhi_controller
Re: [PATCH] drm/virtio: Enable fb damage clips property for the primary plane
Javier Martinez Canillas writes: > Christian Hergert reports that the driver doesn't enable the property and > that leads to always doing a full plane update, even when the driver does > support damage clipping for the primary plane. > > Don't enable it for the cursor plane, because its .atomic_update callback > doesn't handle damage clips. > > Reported-by: Christian Hergert > Signed-off-by: Javier Martinez Canillas > --- Pushed to drm-misc (drm-misc-next). Thanks! -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
[PATCH 2/3] drm/i915/active: Serialize use of barriers as fence trackers
When adding a request to a composite tracker, we try to use an existing
fence tracker already registered with that composite. The tracker we
obtain can already track another fence, can be an idle barrier, or an
active barrier.
When we acquire an idle barrier, we don't claim it in any way until
__i915_active_fence_set() we call substitutes its NULL fence pointer with
that of our request's fence. But another thread looking for an idle
barrier can race with us. If that thread is collecting barriers for
preallocation, it may update the NULL fence pointer with ERR_PTR(-EAGAIN)
barrier mark, either before or after we manage to replace it with our
request fence. It can also corrupt our callback list pointers when
reusing them as an engine pointer (prev) and a preallocated barriers
llist node link (next), or we can corrupt their data.
When we acquire a non-idle barrier in turn, we try to delete that barrier
from a list of barrier tasks it belongs to. If that deletion succeedes
then we convert the barrier to an idle one by replacing its barrier mark
with NULL and decermenting active count of its hosting composite tracker.
But as soon as we do this, we expose that barrier to the above described
idle barrier race.
Claim acquired idle barrier right away by marking it immediately with
ERR_PTR(-EAGAIN) barrier mark. Serialize that operation with other
threads trying to claim a barrier and go back for picking up another
tracker if some other thread wins the race.
Furthermore, on successful deletion of a non-idle barrier from a barrier
tasks list, don't overwrite the barrier mark with NULL -- that's not
needed at the moment since the barrier, once deleted from its list, can no
longer be acquired by any other thread as long as all threads respect
deletion results. Also, don't decrease active counter of the hosting
composite tracker, but skip the follow up step that increases it back.
For the above to work correctly, teach __i915_active_fence_set() function
to recognize and handle non-idle barriers correctly when requested.
The issue has never been reproduced cleanly, only identified via code
analysis while working on fence callback list corruptions which occurred
to have a complex root cause, see commit e0e6b416b25e ("drm/i915/active:
Fix misuse of non-idle barriers as fence trackers") for details. However,
it has been assumed that the issue could start to be potentially
reproducible as soon as timeline mutex locks around calls to
i915_active_fence_set() were dropped by commit df9f85d8582e ("drm/i915:
Serialise i915_active_fence_set() with itself").
Fixes: df9f85d8582e ("drm/i915: Serialise i915_active_fence_set() with itself")
Cc: Chris Wilson
Cc: sta...@vger.kernel.org # v5.6+
Signed-off-by: Janusz Krzysztofik
---
drivers/gpu/drm/i915/i915_active.c | 65 --
1 file changed, 44 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c
b/drivers/gpu/drm/i915/i915_active.c
index b2f79f5c257a8..8eb10af7928f4 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -425,11 +425,17 @@ replace_barrier(struct i915_active *ref, struct
i915_active_fence *active)
return __active_del_barrier(ref, node_from_active(active));
}
+static inline bool is_idle_barrier(struct active_node *node, u64 idx);
+static struct dma_fence *
+i915_active_fence_set(struct i915_active_fence *active,
+ struct dma_fence *fence, bool barrier);
+
int i915_active_add_request(struct i915_active *ref, struct i915_request *rq)
{
u64 idx = i915_request_timeline(rq)->fence_context;
struct dma_fence *fence = >fence;
struct i915_active_fence *active;
+ bool replaced;
int err;
/* Prevent reaping in case we malloc/wait while building the tree */
@@ -444,13 +450,18 @@ int i915_active_add_request(struct i915_active *ref,
struct i915_request *rq)
goto out;
}
- if (replace_barrier(ref, active)) {
- RCU_INIT_POINTER(active->fence, NULL);
- atomic_dec(>count);
- }
- } while (unlikely(is_barrier(active)));
+ replaced = replace_barrier(ref, active);
+ if (replaced)
+ break;
+
+ if (!cmpxchg(__active_fence_slot(active), NULL,
+ERR_PTR(-EAGAIN)))
+ break;
- if (!__i915_active_fence_set(active, fence))
+ } while (IS_ERR_OR_NULL(rcu_access_pointer(active->fence)));
+
+ if (!i915_active_fence_set(active, fence, is_barrier(active)) &&
+ !replaced)
__i915_active_acquire(ref);
out:
@@ -1021,21 +1032,9 @@ void i915_request_add_active_barriers(struct
i915_request *rq)
spin_unlock_irqrestore(>lock, flags);
}
-/*
- * __i915_active_fence_set: Update the last active fence along its timeline
- * @active:
[PATCH 3/3] drm/i915/active: Simplify llist search-and-delete
Inside active_del_barrier(), while searching for a node to be deleted,
we now rebuild barrier_tasks llist content in reverse order.
Theoretically neutral, that method was observed to provide an undocumented
workaround for unexpected loops of llist nodes appearing now and again due
to races, silently breaking those llist node loops, then protecting
llist_for_each_safe() from spinning indefinitely.
Having all races hopefully fixed, make that function behavior more
predictable, more easy to follow -- switch to an alternative, equally
simple but less invasive algorithm that only updates a link between list
nodes that precede and follow the deleted node.
Signed-off-by: Janusz Krzysztofik
---
drivers/gpu/drm/i915/i915_active.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c
b/drivers/gpu/drm/i915/i915_active.c
index 8eb10af7928f4..10f52eb4a4592 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -391,13 +391,14 @@ static bool active_del_barrier(struct i915_active
*ref,
llist_for_each_safe(pos, next, llist_del_all(>barrier_tasks)) {
if (node == barrier_from_ll(pos)) {
node = NULL;
+ if (tail)
+ tail->next = next;
continue;
}
- pos->next = head;
- head = pos;
- if (!tail)
- tail = pos;
+ if (!head)
+ head = pos;
+ tail = pos;
}
if (head)
llist_add_batch(head, tail, >barrier_tasks);
--
2.25.1
[PATCH 1/3] drm/i915/active: Serialize preallocation of idle barriers
When we collect barriers for preallocating them, we reuse either idle or
non-idle ones, whichever we find. In case of non-idle barriers, we
depend on their successful deletion from their barrier tasks lists as an
indication of them not being claimed by another thread. However, in case
of idle barriers, we neither perform any similar checks nor take any
preventive countermeasures against unexpected races with other threads.
We may then end up adding the same barrier to two independent preallocated
lists, and then adding it twice back to the same engine's barrier tasks
list, thus effectively creating a loop of llist nodes. As a result,
searches through that barrier tasks llist may start spinning indefinitely.
Occurrences of that issue were never observed on CI nor reported by users.
However, deep code analysis revealed a silent, most probably not intended
workaround that actively breaks those loops by rebuilding barrier tasks
llists in reverse order inside our local implementation of llist node
deletion. A simple patch that replaces that reverse order rebuild with
just an update of next pointer of a node preceding the one to be deleted
helps to reproduce the race, though still not easily. As soon as we have
the race fixed, we may want to consider such update for the code to be
more clear and more predictable.
To fix the issue, whenever an idle barrier is selected for preallocation,
mark it immediately as non-idle with our ERR_PTR(-EAGAIN) barrier mark, so
other threads are no longer able to claim it, neither as idle nor as
non-idle since not a member of respective barrier tasks list. Serialize
that claim operation against other potential concurrent updates of active
fence pointer, and skip the node in favor of allocating a new one if it
occurs claimed meanwhile by another competing thread. Once claimed,
increase active count of its composite tracker host immediately, as long
as we still know that was an idle barrier.
While being at it, fortify now still marginally racy check for
preallocated_barriers llist being still empty when we populate it with
collected proto-barriers (assuming we need that check).
Fixes: 9ff33bbcda25 ("drm/i915: Reduce locking around
i915_active_acquire_preallocate_barrier()")
Cc: Chris Wilson
Cc: sta...@vger.kernel.org # v5.10+
Signed-off-by: Janusz Krzysztofik
---
drivers/gpu/drm/i915/i915_active.c | 50 +-
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c
b/drivers/gpu/drm/i915/i915_active.c
index a9fea115f2d26..b2f79f5c257a8 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -788,8 +788,13 @@ static struct active_node *reuse_idle_barrier(struct
i915_active *ref, u64 idx)
* node kept alive (as we reuse before parking). We prefer to reuse
* completely idle barriers (less hassle in manipulating the llists),
* but otherwise any will do.
+*
+* We reuse the request field to mark this as being our proto-node.
*/
- if (ref->cache && is_idle_barrier(ref->cache, idx)) {
+ if (ref->cache && is_idle_barrier(ref->cache, idx) &&
+ !cmpxchg(__active_fence_slot(>cache->base), NULL,
+ERR_PTR(-EAGAIN))) {
+ __i915_active_acquire(ref);
p = >cache->node;
goto match;
}
@@ -800,8 +805,12 @@ static struct active_node *reuse_idle_barrier(struct
i915_active *ref, u64 idx)
struct active_node *node =
rb_entry(p, struct active_node, node);
- if (is_idle_barrier(node, idx))
+ if (is_idle_barrier(node, idx) &&
+ !cmpxchg(__active_fence_slot(>base), NULL,
+ERR_PTR(-EAGAIN))) {
+ __i915_active_acquire(ref);
goto match;
+ }
prev = p;
if (node->timeline < idx)
@@ -827,8 +836,12 @@ static struct active_node *reuse_idle_barrier(struct
i915_active *ref, u64 idx)
if (node->timeline < idx)
continue;
- if (is_idle_barrier(node, idx))
+ if (is_idle_barrier(node, idx) &&
+ !cmpxchg(__active_fence_slot(>base), NULL,
+ERR_PTR(-EAGAIN))) {
+ __i915_active_acquire(ref);
goto match;
+ }
/*
* The list of pending barriers is protected by the
@@ -889,29 +902,24 @@ int i915_active_acquire_preallocate_barrier(struct
i915_active *ref,
if (!node)
goto unwind;
- RCU_INIT_POINTER(node->base.fence, NULL);
+ /* Mark this as being our unconnected proto-node */
+ RCU_INIT_POINTER(node->base.fence, ERR_PTR(-EAGAIN));
[PATCH 0/3] drm/i915/active: Fix other potential list corruption root causes
While perfroming root cause analyses of fence callback list corruptions, a couple of other potential though less likely root causes have been identified in addition to barrier tasks list deletion results ignored. This series tries to fix those potential issues, also in longterm stable releases starting from v5.10. The third patch, while not fixing any real bug, is believed to make the code more predictable and easy to understand, then more easy to debug should other barrier related issue still exist. Janusz Krzysztofik (3): drm/i915/active: Serialize preallocation of idle barriers drm/i915/active: Serialize use of barriers as fence trackers drm/i915/active: Simplify llist search-and-delete drivers/gpu/drm/i915/i915_active.c | 124 ++--- 1 file changed, 78 insertions(+), 46 deletions(-) -- 2.25.1
Re: [PATCH] backlight: apple_bl: Use acpi_video_get_backlight_type()
On Thu, 09 Mar 2023, Daniel Thompson wrote: > On Tue, Mar 07, 2023 at 01:05:40PM +0100, Hans de Goede wrote: > > On some MacBooks both the apple_bl and the apple-gmux backlight drivers > > may be able to export a /sys/class/backlight device. > > > > To avoid having 2 backlight devices for one LCD panel until now > > the apple-gmux driver has been calling apple_bl_unregister() to move > > the apple_bl backlight device out of the way when it loads. > > > > Similar problems exist on other x86 laptops and all backlight drivers > > which may be used on x86 laptops have moved to using > > acpi_video_get_backlight_type() to determine whether they should load > > or not. > > > > Switch apple_bl to this model too, so that it is consistent with all > > the other x86 backlight drivers. > > [snip] > > Signed-off-by: Hans de Goede > > Reviewed-by: Daniel Thompson > > but... > > > --- > > A note to the backlight class / subsystem maintainers, this change > > applies on top of a similar patch for drivers/platform/x86/apple-gmux.c > > which makes that driver use acpi_video_get_backlight_type(). See: > > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans > > > > I believe it is easiest to also merge this patch through > > the platform-drivers-x86 tree, may I please have your Ack for this ? > > --- > > ... please don't treat above as an ack. Lee Jones will hopefully be > along shortly to discuss that! That's fine. I will need a succinct, immutable pull-request though. Acked-by: Lee Jones -- Lee Jones [李琼斯]
Re: [Freedreno] [PATCH v3 4/7] drm/msm/a2xx: Implement .gpu_busy
On 24.02.2023 16:04, Jonathan Marek wrote:
> This won't work because a2xx freedreno userspace expects to own all the
> perfcounters.
>
> This will break perfcounters for userspace, and when userspace isn't using
> perfcounters, this won't count correctly because userspace writes 0 to
> CP_PERFMON_CNTL at the start of every submit.
Rob, would you be willing to take this without the a2xx bits? It
should still be fine, except without devfreq. Not that we had
any significant sort of scaling on a2xx before.
Konrad
>
> On 2/23/23 5:52 AM, Konrad Dybcio wrote:
>> Implement gpu_busy based on the downstream msm-3.4 code [1]. This
>> allows us to use devfreq on this old old old hardware!
>>
>> [1]
>> https://github.com/LineageOS/android_kernel_sony_apq8064/blob/lineage-16.0/drivers/gpu/msm/adreno_a2xx.c#L1975
>>
>> Reviewed-by: Dmitry Baryshkov
>> Signed-off-by: Konrad Dybcio
>> ---
>> drivers/gpu/drm/msm/adreno/a2xx_gpu.c | 26 ++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
>> b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
>> index c67089a7ebc1..104bdf28cdaf 100644
>> --- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
>> @@ -481,6 +481,31 @@ a2xx_create_address_space(struct msm_gpu *gpu, struct
>> platform_device *pdev)
>> return aspace;
>> }
>> +/* While the precise size of this field is unknown, it holds at least
>> these three values.. */
>> +static u64 a2xx_gpu_busy(struct msm_gpu *gpu, unsigned long
>> *out_sample_rate)
>> +{
>> + u64 busy_cycles;
>> +
>> + /* Freeze the counter */
>> + gpu_write(gpu, REG_A2XX_CP_PERFMON_CNTL, PERF_STATE_FREEZE);
>> +
>> + busy_cycles = gpu_read64(gpu, REG_A2XX_RBBM_PERFCOUNTER1_LO);
>> +
>> + /* Reset the counter */
>> + gpu_write(gpu, REG_A2XX_CP_PERFMON_CNTL, PERF_STATE_RESET);
>> +
>> + /* Re-enable the performance monitors */
>> + gpu_rmw(gpu, REG_A2XX_RBBM_PM_OVERRIDE2,
>> + A2XX_RBBM_PM_OVERRIDE2_DEBUG_PERF_SCLK_PM_OVERRIDE,
>> + A2XX_RBBM_PM_OVERRIDE2_DEBUG_PERF_SCLK_PM_OVERRIDE);
>> + gpu_write(gpu, REG_A2XX_RBBM_PERFCOUNTER1_SELECT, 1);
>> + gpu_write(gpu, REG_A2XX_CP_PERFMON_CNTL, PERF_STATE_ENABLE);
>> +
>> + *out_sample_rate = clk_get_rate(gpu->core_clk);
>> +
>> + return busy_cycles;
>> +}
>> +
>> static u32 a2xx_get_rptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
>> {
>> ring->memptrs->rptr = gpu_read(gpu, REG_AXXX_CP_RB_RPTR);
>> @@ -502,6 +527,7 @@ static const struct adreno_gpu_funcs funcs = {
>> #if defined(CONFIG_DEBUG_FS) || defined(CONFIG_DEV_COREDUMP)
>> .show = adreno_show,
>> #endif
>> + .gpu_busy = a2xx_gpu_busy,
>> .gpu_state_get = a2xx_gpu_state_get,
>> .gpu_state_put = adreno_gpu_state_put,
>> .create_address_space = a2xx_create_address_space,
>>
Re: [Intel-xe] [PATCH 1/2] drm/xe: Introduce GT oriented log messages
+dri-devel On Mon, Mar 13, 2023 at 09:03:56AM +0100, Michal Wajdeczko wrote: While debugging GT related problems, it's good to know which GT was reporting problems. Introduce helper macros to allow prefix GT logs with GT idetifier. We will use them in upcoming patches. Signed-off-by: Michal Wajdeczko --- drivers/gpu/drm/xe/xe_gt_printk.h | 45 +++ 1 file changed, 45 insertions(+) create mode 100644 drivers/gpu/drm/xe/xe_gt_printk.h diff --git a/drivers/gpu/drm/xe/xe_gt_printk.h b/drivers/gpu/drm/xe/xe_gt_printk.h new file mode 100644 index ..b12a92024126 --- /dev/null +++ b/drivers/gpu/drm/xe/xe_gt_printk.h @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2023 Intel Corporation + */ + +#ifndef _XE_GT_PRINTK_H_ +#define _XE_GT_PRINTK_H_ + +#include missing blank line +#include "xe_gt_types.h" + +#define gt_printk(_gt, _level, _fmt, ...) \ any exposed interface in headers should have xe_ prefix. I myself find it odd to have these macros that are wrappers over wrappers and create a silo with xe-speficic debugging macros. The DRM ones at least are shared across drivers. The pr_dbg(), just reusing a local define is nice as it's still the same call used everywhere... but it wouldn't work very well here as we need the extra parm. I won't oppose them though since a lot of people seem to like the approach to help their debug and i915 went through similar discussion. Lucas De Marchi + drm_##_level(&(_gt)->xe->drm, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) + +#define gt_err(_gt, _fmt, ...) \ + gt_printk((_gt), err, _fmt, ##__VA_ARGS__) + +#define gt_warn(_gt, _fmt, ...) \ + gt_printk((_gt), warn, _fmt, ##__VA_ARGS__) + +#define gt_notice(_gt, _fmt, ...) \ + gt_printk((_gt), notice, _fmt, ##__VA_ARGS__) + +#define gt_info(_gt, _fmt, ...) \ + gt_printk((_gt), info, _fmt, ##__VA_ARGS__) + +#define gt_dbg(_gt, _fmt, ...) \ + gt_printk((_gt), dbg, _fmt, ##__VA_ARGS__) + +#define gt_err_ratelimited(_gt, _fmt, ...) \ + gt_printk((_gt), err_ratelimited, _fmt, ##__VA_ARGS__) + +#define gt_WARN(_gt, _condition, _fmt, ...) \ + drm_WARN(&(_gt)->xe->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) + +#define gt_WARN_ONCE(_gt, _condition, _fmt, ...) \ + drm_WARN_ONCE(&(_gt)->xe->drm, _condition, "GT%u: " _fmt, (_gt)->info.id, ##__VA_ARGS__) + +#define gt_WARN_ON(_gt, _condition) \ + gt_WARN((_gt), _condition, "%s(%s)", "gt_WARN_ON", __stringify(_condition)) + +#define gt_WARN_ON_ONCE(_gt, _condition) \ + gt_WARN_ONCE((_gt), _condition, "%s(%s)", "gt_WARN_ON_ONCE", __stringify(_condition)) + +#endif -- 2.25.1
Re: [PATCH 2/2] drm/msm: Embed the hw_fence in msm_gem_submit
On Mon, Mar 13, 2023 at 9:15 AM Christian König
wrote:
>
> Am 13.03.23 um 15:45 schrieb Rob Clark:
> > On Mon, Mar 13, 2023 at 12:19 AM Christian König
> > wrote:
> >> Am 11.03.23 um 18:35 schrieb Rob Clark:
> >>> From: Rob Clark
> >>>
> >>> Avoid allocating memory in job_run() by embedding the fence in the
> >>> submit object. Since msm gpu fences are always 1:1 with msm_gem_submit
> >>> we can just use the fence's refcnt to track the submit. And since we
> >>> can get the fence ctx from the submit we can just drop the msm_fence
> >>> struct altogether. This uses the new dma_fence_init_noref() to deal
> >>> with the fact that the fence's refcnt is initialized when the submit is
> >>> created, long before job_run().
> >> Well this is a very very bad idea, we made the same mistake with amdgpu
> >> as well.
> >>
> >> It's true that you should not have any memory allocation in your run_job
> >> callback, but you could also just allocate the hw fence during job
> >> creation and initializing it later on.
> >>
> >> I've suggested to embed the fence into the job for amdgpu because some
> >> people insisted of re-submitting jobs during timeout and GPU reset. This
> >> turned into a nightmare with tons of bug fixes on top of bug fixes on
> >> top of bug fixes because it messes up the job and fence lifetime as
> >> defined by the DRM scheduler and DMA-buf framework.
> >>
> >> Luben is currently working on cleaning all this up.
> > This actually shouldn't be a problem with msm, as the fence doesn't
> > change if there is a gpu reset. We simply signal the fence for the
> > offending job, reset the GPU, and re-play the remaining in-flight jobs
> > (ie. things that already had their job_run() called) with the original
> > fences. (We don't use gpu sched's reset/timeout handling.. when I
> > migrated to gpu sched I kept our existing hangcheck/recovery
> > mechanism.)
>
> That sounds much saner than what we did.
>
> So you basically need the dma_fence reference counting separate to
> initializing the other dma_fence fields?
yeah, that was the idea
> What would happen if a dma_fence which is not completely initialized
> gets freed? E.g. because of an error?
hmm, yes, this would be a problem since ops->release is not set yet..
and I'm relying on that to free the submit
> Would it be to much to just keep the handling as it is today and only
> allocate the dma_fence without initializing it? If necessary we could
> easily add a dma_fence_is_initialized() function which checks the
> fence_ops for NULL.
Yeah, that would also be possible
I guess we could split creation of the fence (initializing ops,
refcount) and "arming" it later when the seqno is known? But maybe
that is going to too many lengths to avoid a separate allocation..
BR,
-R
>
> Thanks,
> Christian.
>
> >
> > BR,
> > -R
> >
> >> Regards,
> >> Christian.
> >>
> >>> Signed-off-by: Rob Clark
> >>> ---
> >>> Note that this applies on top of
> >>> https://patchwork.freedesktop.org/series/93035/
> >>> out of convenience for myself, but I can re-work it to go before
> >>> depending on the order that things land.
> >>>
> >>>drivers/gpu/drm/msm/msm_fence.c | 45 +++-
> >>>drivers/gpu/drm/msm/msm_fence.h | 2 +-
> >>>drivers/gpu/drm/msm/msm_gem.h| 10 +++
> >>>drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++---
> >>>drivers/gpu/drm/msm/msm_gpu.c| 4 +--
> >>>drivers/gpu/drm/msm/msm_ringbuffer.c | 4 +--
> >>>6 files changed, 31 insertions(+), 42 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/msm_fence.c
> >>> b/drivers/gpu/drm/msm/msm_fence.c
> >>> index 51b461f32103..51f9f1f0cb66 100644
> >>> --- a/drivers/gpu/drm/msm/msm_fence.c
> >>> +++ b/drivers/gpu/drm/msm/msm_fence.c
> >>> @@ -103,14 +103,9 @@ void msm_update_fence(struct msm_fence_context
> >>> *fctx, uint32_t fence)
> >>>spin_unlock_irqrestore(>spinlock, flags);
> >>>}
> >>>
> >>> -struct msm_fence {
> >>> - struct dma_fence base;
> >>> - struct msm_fence_context *fctx;
> >>> -};
> >>> -
> >>> -static inline struct msm_fence *to_msm_fence(struct dma_fence *fence)
> >>> +static inline struct msm_gem_submit *fence_to_submit(struct dma_fence
> >>> *fence)
> >>>{
> >>> - return container_of(fence, struct msm_fence, base);
> >>> + return container_of(fence, struct msm_gem_submit, hw_fence);
> >>>}
> >>>
> >>>static const char *msm_fence_get_driver_name(struct dma_fence *fence)
> >>> @@ -120,20 +115,20 @@ static const char *msm_fence_get_driver_name(struct
> >>> dma_fence *fence)
> >>>
> >>>static const char *msm_fence_get_timeline_name(struct dma_fence *fence)
> >>>{
> >>> - struct msm_fence *f = to_msm_fence(fence);
> >>> - return f->fctx->name;
> >>> + struct msm_gem_submit *submit = fence_to_submit(fence);
> >>> + return submit->ring->fctx->name;
> >>>}
> >>>
> >>>static bool msm_fence_signaled(struct dma_fence *fence)
> >>>{
> >>> -
[PATCH v3 1/2] drm: Introduce plane SIZE_HINTS property
From: Ville Syrjälä
Add a new immutable plane property by which a plane can advertise
a handful of recommended plane sizes. This would be mostly exposed
by cursor planes as a slightly more capable replacement for
the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare
a one size fits all limit for the whole device.
Currently eg. amdgpu/i915/nouveau just advertize the max cursor
size via the cursor size caps. But always using the max sized
cursor can waste a surprising amount of power, so a better
stragey is desirable.
Most other drivers don't specify any cursor size at all, in
which case the ioctl code just claims that 64x64 is a great
choice. Whether that is actually true is debatable.
A poll of various compositor developers informs us that
blindly probing with setcursor/atomic ioctl to determine
suitable cursor sizes is not acceptable, thus the
introduction of the new property to supplant the cursor
size caps. The compositor will now be free to select a
more optimal cursor size from the short list of options.
Note that the reported sizes (either via the property or the
caps) make no claims about things such as plane scaling. So
these things should only really be consulted for simple
"cursor like" use cases.
v2: Try to add some docs
v3: Specify that value 0 is reserved for future use (basic idea from Jonas)
Drop the note about typical hardware (Pekka)
Cc: Simon Ser
Cc: Jonas Ådahl
Cc: Daniel Stone
Cc: Pekka Paalanen
Acked-by: Harry Wentland
Acked-by: Pekka Paalanen
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_mode_config.c | 7
drivers/gpu/drm/drm_plane.c | 53 +++
include/drm/drm_mode_config.h | 5 +++
include/drm/drm_plane.h | 4 +++
include/uapi/drm/drm_mode.h | 11 +++
5 files changed, 80 insertions(+)
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index 87eb591fe9b5..21860f94a18c 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.modifiers_property = prop;
+ prop = drm_property_create(dev,
+ DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
+ "SIZE_HINTS", 0);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.size_hints_property = prop;
+
return 0;
}
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 24e7998d1731..d2a6fdff1a57 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -140,6 +140,26 @@
* DRM_FORMAT_MOD_LINEAR. Before linux kernel release v5.1 there have been
* various bugs in this area with inconsistencies between the capability
* flag and per-plane properties.
+ *
+ * SIZE_HINTS:
+ * Blob property which contains the set of recommended plane size
+ * which can used for simple "cursor like" use cases (eg. no scaling).
+ * Using these hints frees userspace from extensive probing of
+ * supported plane sizes through atomic/setcursor ioctls.
+ *
+ * For optimal usage userspace should pick the smallest size
+ * that satisfies its own requirements.
+ *
+ * The blob contains an array of struct drm_plane_size_hint.
+ *
+ * Drivers should only attach this property to planes that
+ * support a very limited set of sizes.
+ *
+ * Note that property value 0 (ie. no blob) is reserved for potential
+ * future use. Current userspace is expected to ignore the property
+ * if the value is 0, and fall back to some other means (eg.
+ * _CAP_CURSOR_WIDTH and _CAP_CURSOR_HEIGHT) to determine
+ * the appropriate plane size to use.
*/
static unsigned int drm_num_planes(struct drm_device *dev)
@@ -1582,3 +1602,36 @@ int drm_plane_create_scaling_filter_property(struct
drm_plane *plane,
return 0;
}
EXPORT_SYMBOL(drm_plane_create_scaling_filter_property);
+
+/**
+ * drm_plane_add_size_hint_property - create a size hint property
+ *
+ * @plane: drm plane
+ * @hints: size hints
+ * @num_hints: number of size hints
+ *
+ * Create a size hints property for the plane.
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+int drm_plane_add_size_hints_property(struct drm_plane *plane,
+ const struct drm_plane_size_hint *hints,
+ int num_hints)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_mode_config *config = >mode_config;
+ struct drm_property_blob *blob;
+
+ blob = drm_property_create_blob(dev,
+ array_size(sizeof(hints[0]), num_hints),
+ hints);
+ if (IS_ERR(blob))
+ return PTR_ERR(blob);
+
+ drm_object_attach_property(>base, config->size_hints_property,
+
Re: [PATCH 2/2] drm/msm: Embed the hw_fence in msm_gem_submit
Am 13.03.23 um 15:45 schrieb Rob Clark:
On Mon, Mar 13, 2023 at 12:19 AM Christian König
wrote:
Am 11.03.23 um 18:35 schrieb Rob Clark:
From: Rob Clark
Avoid allocating memory in job_run() by embedding the fence in the
submit object. Since msm gpu fences are always 1:1 with msm_gem_submit
we can just use the fence's refcnt to track the submit. And since we
can get the fence ctx from the submit we can just drop the msm_fence
struct altogether. This uses the new dma_fence_init_noref() to deal
with the fact that the fence's refcnt is initialized when the submit is
created, long before job_run().
Well this is a very very bad idea, we made the same mistake with amdgpu
as well.
It's true that you should not have any memory allocation in your run_job
callback, but you could also just allocate the hw fence during job
creation and initializing it later on.
I've suggested to embed the fence into the job for amdgpu because some
people insisted of re-submitting jobs during timeout and GPU reset. This
turned into a nightmare with tons of bug fixes on top of bug fixes on
top of bug fixes because it messes up the job and fence lifetime as
defined by the DRM scheduler and DMA-buf framework.
Luben is currently working on cleaning all this up.
This actually shouldn't be a problem with msm, as the fence doesn't
change if there is a gpu reset. We simply signal the fence for the
offending job, reset the GPU, and re-play the remaining in-flight jobs
(ie. things that already had their job_run() called) with the original
fences. (We don't use gpu sched's reset/timeout handling.. when I
migrated to gpu sched I kept our existing hangcheck/recovery
mechanism.)
That sounds much saner than what we did.
So you basically need the dma_fence reference counting separate to
initializing the other dma_fence fields?
What would happen if a dma_fence which is not completely initialized
gets freed? E.g. because of an error?
Would it be to much to just keep the handling as it is today and only
allocate the dma_fence without initializing it? If necessary we could
easily add a dma_fence_is_initialized() function which checks the
fence_ops for NULL.
Thanks,
Christian.
BR,
-R
Regards,
Christian.
Signed-off-by: Rob Clark
---
Note that this applies on top of https://patchwork.freedesktop.org/series/93035/
out of convenience for myself, but I can re-work it to go before
depending on the order that things land.
drivers/gpu/drm/msm/msm_fence.c | 45 +++-
drivers/gpu/drm/msm/msm_fence.h | 2 +-
drivers/gpu/drm/msm/msm_gem.h| 10 +++
drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++---
drivers/gpu/drm/msm/msm_gpu.c| 4 +--
drivers/gpu/drm/msm/msm_ringbuffer.c | 4 +--
6 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 51b461f32103..51f9f1f0cb66 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -103,14 +103,9 @@ void msm_update_fence(struct msm_fence_context *fctx,
uint32_t fence)
spin_unlock_irqrestore(>spinlock, flags);
}
-struct msm_fence {
- struct dma_fence base;
- struct msm_fence_context *fctx;
-};
-
-static inline struct msm_fence *to_msm_fence(struct dma_fence *fence)
+static inline struct msm_gem_submit *fence_to_submit(struct dma_fence *fence)
{
- return container_of(fence, struct msm_fence, base);
+ return container_of(fence, struct msm_gem_submit, hw_fence);
}
static const char *msm_fence_get_driver_name(struct dma_fence *fence)
@@ -120,20 +115,20 @@ static const char *msm_fence_get_driver_name(struct
dma_fence *fence)
static const char *msm_fence_get_timeline_name(struct dma_fence *fence)
{
- struct msm_fence *f = to_msm_fence(fence);
- return f->fctx->name;
+ struct msm_gem_submit *submit = fence_to_submit(fence);
+ return submit->ring->fctx->name;
}
static bool msm_fence_signaled(struct dma_fence *fence)
{
- struct msm_fence *f = to_msm_fence(fence);
- return msm_fence_completed(f->fctx, f->base.seqno);
+ struct msm_gem_submit *submit = fence_to_submit(fence);
+ return msm_fence_completed(submit->ring->fctx, fence->seqno);
}
static void msm_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)
{
- struct msm_fence *f = to_msm_fence(fence);
- struct msm_fence_context *fctx = f->fctx;
+ struct msm_gem_submit *submit = fence_to_submit(fence);
+ struct msm_fence_context *fctx = submit->ring->fctx;
unsigned long flags;
ktime_t now;
@@ -165,26 +160,22 @@ static void msm_fence_set_deadline(struct dma_fence
*fence, ktime_t deadline)
spin_unlock_irqrestore(>spinlock, flags);
}
+static void msm_fence_release(struct dma_fence *fence)
+{
+ __msm_gem_submit_destroy(fence_to_submit(fence));
+}
+
static const struct dma_fence_ops msm_fence_ops = {
.get_driver_name =
[PATCH v2 18/25] drm/tilcdc: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 4ca426007dc8..fe56beea3e93 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include #include @@ -384,7 +384,7 @@ static int tilcdc_init(const struct drm_driver *ddrv, struct device *dev) goto init_failed; priv->is_registered = true; - drm_fbdev_generic_setup(ddev, bpp); + drm_fbdev_dma_setup(ddev, bpp); return 0; init_failed: -- 2.39.2
[PATCH v2 24/25] drm/mcde: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Reported-by: Linus Walleij Link: https://lore.kernel.org/dri-devel/cacrpkdawsqsnqkjkssosw3hmmhynxfuywxkdszptc-a_uza...@mail.gmail.com/ Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mcde/mcde_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c index a592ad0d7886..a8cd86c06c14 100644 --- a/drivers/gpu/drm/mcde/mcde_drv.c +++ b/drivers/gpu/drm/mcde/mcde_drv.c @@ -69,7 +69,7 @@ #include #include #include -#include +#include #include #include #include @@ -237,7 +237,7 @@ static int mcde_drm_bind(struct device *dev) if (ret < 0) goto unbind; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); return 0; -- 2.39.2
[PATCH v2 25/25] drm/pl111: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Reported-by: Linus Walleij Link: https://lore.kernel.org/dri-devel/cacrpkdawsqsnqkjkssosw3hmmhynxfuywxkdszptc-a_uza...@mail.gmail.com/ Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/pl111/pl111_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index 00deba0b7271..4b2a9e9753f6 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -48,7 +48,7 @@ #include #include #include -#include +#include #include #include #include @@ -308,7 +308,7 @@ static int pl111_amba_probe(struct amba_device *amba_dev, if (ret < 0) goto dev_put; - drm_fbdev_generic_setup(drm, priv->variant->fb_bpp); + drm_fbdev_dma_setup(drm, priv->variant->fb_bpp); return 0; -- 2.39.2
[PATCH v2 07/25] drm/imx/dcss: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/imx/dcss/dcss-kms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c b/drivers/gpu/drm/imx/dcss/dcss-kms.c index dab5e664920d..896de946f8df 100644 --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include #include @@ -145,7 +145,7 @@ struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss) if (ret) goto cleanup_crtc; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); return kms; -- 2.39.2
[PATCH v2 05/25] drm/atmel-hlcdc: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Acked-by: Sam Ravnborg --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 4e806b06d35d..29603561d501 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include @@ -760,7 +760,7 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev) if (ret) goto err_unload; - drm_fbdev_generic_setup(ddev, 24); + drm_fbdev_dma_setup(ddev, 24); return 0; -- 2.39.2
[PATCH v2 02/25] arm/hdlcd: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/arm/hdlcd_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index e3507dd6f82a..a554c79dcd39 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include #include @@ -301,7 +301,7 @@ static int hdlcd_drm_bind(struct device *dev) if (ret) goto err_register; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); return 0; -- 2.39.2
[PATCH v2 04/25] drm/aspeed: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index ecfb060d2557..c8c7f8215155 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -15,7 +15,7 @@ #include #include -#include +#include #include #include #include @@ -341,7 +341,7 @@ static int aspeed_gfx_probe(struct platform_device *pdev) if (ret) goto err_unload; - drm_fbdev_generic_setup(>drm, 32); + drm_fbdev_dma_setup(>drm, 32); return 0; err_unload: -- 2.39.2
[PATCH v2 21/25] drm/vc4: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/vc4/vc4_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 0ccaee57fe9a..c8bf954042e0 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -33,7 +33,7 @@ #include #include #include -#include +#include #include #include @@ -387,7 +387,7 @@ static int vc4_drm_bind(struct device *dev) if (ret < 0) goto unbind_all; - drm_fbdev_generic_setup(drm, 16); + drm_fbdev_dma_setup(drm, 16); return 0; -- 2.39.2
[PATCH v2 23/25] drm/mcde: Do not use dirty GEM FB handling
From: Linus Walleij
This driver has no way to handle damage, the reason the
drm_gem_fb_create_with_dirty() was used was because I had the
ambition that the driver would only send out updates to DSI
command displays whenever something changed, so as to
minimize traffic.
It turns out this ambition with command mode isn't working
in practice because all the MCDE does is to create a
continuous stream of DSI commands and while it is possible to
send single frame updates with it, it's not been worthwhile.
So we are just setting up continuous updates.
Reported-by: Thomas Zimmermann
Link:
https://lore.kernel.org/dri-devel/0e789778-03ca-e3cb-9c94-e8b555738...@suse.de/
Signed-off-by: Linus Walleij
Reviewed-by: Thomas Zimmermann
---
drivers/gpu/drm/mcde/mcde_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index 4aedb050d2a5..a592ad0d7886 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -94,7 +94,7 @@
#define MCDE_PID_MAJOR_VERSION_MASK 0xFF00
static const struct drm_mode_config_funcs mcde_mode_config_funcs = {
- .fb_create = drm_gem_fb_create_with_dirty,
+ .fb_create = drm_gem_fb_create,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = drm_atomic_helper_commit,
};
--
2.39.2
[PATCH v2 22/25] drm/xlnx: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/xlnx/zynqmp_kms.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_kms.c b/drivers/gpu/drm/xlnx/zynqmp_kms.c index 776ef5480206..a7f8611be6f4 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_kms.c +++ b/drivers/gpu/drm/xlnx/zynqmp_kms.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include @@ -515,7 +515,7 @@ int zynqmp_dpsub_drm_init(struct zynqmp_dpsub *dpsub) goto err_poll_fini; /* Initialize fbdev generic emulation. */ - drm_fbdev_generic_setup(drm, 24); + drm_fbdev_dma_setup(drm, 24); return 0; -- 2.39.2
[PATCH v2 12/25] drm/mxsfb/lcdif: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/mxsfb/lcdif_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_drv.c b/drivers/gpu/drm/mxsfb/lcdif_drv.c index cc2ceb301b96..6fb5b469ee5a 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_drv.c +++ b/drivers/gpu/drm/mxsfb/lcdif_drv.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include #include @@ -220,7 +220,7 @@ static int lcdif_probe(struct platform_device *pdev) if (ret) goto err_unload; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); return 0; -- 2.39.2
[PATCH v2 11/25] drm/meson: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Acked-by: Martin Blumenstingl --- drivers/gpu/drm/meson/meson_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c index 79bfe3938d3c..6608a251106b 100644 --- a/drivers/gpu/drm/meson/meson_drv.c +++ b/drivers/gpu/drm/meson/meson_drv.c @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include #include @@ -353,7 +353,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components) if (ret) goto uninstall_irq; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); return 0; -- 2.39.2
[PATCH v2 20/25] drm/tve200: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas Acked-by: Linus Walleij --- drivers/gpu/drm/tve200/tve200_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c index 0d05c386d303..abd557332b28 100644 --- a/drivers/gpu/drm/tve200/tve200_drv.c +++ b/drivers/gpu/drm/tve200/tve200_drv.c @@ -40,7 +40,7 @@ #include #include #include -#include +#include #include #include #include @@ -227,7 +227,7 @@ static int tve200_probe(struct platform_device *pdev) * Passing in 16 here will make the RGB565 mode the default * Passing in 32 will use XRGB mode */ - drm_fbdev_generic_setup(drm, 16); + drm_fbdev_dma_setup(drm, 16); return 0; -- 2.39.2
[PATCH v2 06/25] drm/fsl-dcu: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index 8579c7629f5e..c09ba019ba5e 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -20,7 +20,7 @@ #include #include -#include +#include #include #include #include @@ -333,7 +333,7 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev) if (ret < 0) goto put; - drm_fbdev_generic_setup(drm, legacyfb_depth); + drm_fbdev_dma_setup(drm, legacyfb_depth); return 0; -- 2.39.2
[PATCH v2 19/25] drm/arcpgu: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/tiny/arcpgu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tiny/arcpgu.c b/drivers/gpu/drm/tiny/arcpgu.c index 611bbee15071..e5b10e41554a 100644 --- a/drivers/gpu/drm/tiny/arcpgu.c +++ b/drivers/gpu/drm/tiny/arcpgu.c @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include #include @@ -394,7 +394,7 @@ static int arcpgu_probe(struct platform_device *pdev) if (ret) goto err_unload; - drm_fbdev_generic_setup(>drm, 16); + drm_fbdev_dma_setup(>drm, 16); return 0; -- 2.39.2
[PATCH v2 14/25] drm/sti: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/sti/sti_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index ef6a4e63198f..1b87b5899f9e 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include @@ -199,7 +199,7 @@ static int sti_bind(struct device *dev) drm_mode_config_reset(ddev); - drm_fbdev_generic_setup(ddev, 32); + drm_fbdev_dma_setup(ddev, 32); return 0; -- 2.39.2
[PATCH v2 08/25] drm/imx: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/imx/ipuv3/imx-drm-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c index e060fa6cbcb9..4a866ac60fff 100644 --- a/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c +++ b/drivers/gpu/drm/imx/ipuv3/imx-drm-core.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include #include #include @@ -249,7 +249,7 @@ static int imx_drm_bind(struct device *dev) if (ret) goto err_poll_fini; - drm_fbdev_generic_setup(drm, legacyfb_depth); + drm_fbdev_dma_setup(drm, legacyfb_depth); return 0; -- 2.39.2
[PATCH v2 17/25] drm/tidss: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/tidss/tidss_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c index 2dac8727d2f4..3f5f27fb6ebc 100644 --- a/drivers/gpu/drm/tidss/tidss_drv.c +++ b/drivers/gpu/drm/tidss/tidss_drv.c @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include @@ -179,7 +179,7 @@ static int tidss_probe(struct platform_device *pdev) goto err_irq_uninstall; } - drm_fbdev_generic_setup(ddev, 32); + drm_fbdev_dma_setup(ddev, 32); dev_dbg(dev, "%s done\n", __func__); -- 2.39.2
[PATCH v2 13/25] drm/mxsfb: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index b3ab86ad1b36..368b1fbd8305 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include #include @@ -365,7 +365,7 @@ static int mxsfb_probe(struct platform_device *pdev) if (ret) goto err_unload; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); return 0; -- 2.39.2
[PATCH v2 16/25] drm/sun4i: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/sun4i/sun4i_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index d6c741716167..e49f78a6a8cf 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include #include @@ -111,7 +111,7 @@ static int sun4i_drv_bind(struct device *dev) if (ret) goto finish_poll; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_dma_setup(drm, 32); dev_set_drvdata(dev, drm); -- 2.39.2
[PATCH v2 15/25] drm/stm: Use GEM DMA fbdev emulation
Use the fbdev emulation that is optimized for DMA helpers. Avoids possible shadow buffering and makes the code simpler. Signed-off-by: Thomas Zimmermann Reviewed-by: Javier Martinez Canillas --- drivers/gpu/drm/stm/drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c index 50410bd99dfe..40df7d8c 100644 --- a/drivers/gpu/drm/stm/drv.c +++ b/drivers/gpu/drm/stm/drv.c @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include #include @@ -203,7 +203,7 @@ static int stm_drm_platform_probe(struct platform_device *pdev) if (ret) goto err_put; - drm_fbdev_generic_setup(ddev, 16); + drm_fbdev_dma_setup(ddev, 16); return 0; -- 2.39.2
