[Freedreno] [PATCH v3 00/12] drm/msm/dpu: support virtual wide planes
As promised in the basic wide planes support ([1]) here comes a series supporting 2*max_linewidth for all the planes. Note: Unlike v1 and v2 this series finally includes support for additional planes - having more planes than the number of SSPP blocks. Note: this iteration features handling of rotation and reflection of the wide plane. However rot90 is still not tested: it is enabled on sc7280 and it only supports UBWC (tiled) framebuffers, it was quite low on my priority list. [1] https://patchwork.freedesktop.org/series/99909/ Changes since v2: - Dropped the encoder-related parts, leave all resource allocation as is (Abhinav) - Significantly reworked the SSPP allocation code - Added debugging code to dump RM state in dri/N/state Changes since v1: - Fixed build error due to me missing one of fixups, it was left uncommitted. - Implementated proper handling of wide plane rotation & reflection. Dmitry Baryshkov (12): drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state drm/msm/dpu: add current resource allocation to dumped state drm/msm/dpu: take plane rotation into account for wide planes drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check drm/msm/dpu: split dpu_plane_atomic_check() drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe() drm/msm/dpu: add support for virtual planes drm/msm/dpu: allow using two SSPP blocks for a single plane drm/msm/dpu: allow sharing SSPP between planes drm/msm/dpu: create additional virtual planes drm/msm/dpu: allow sharing of blending stages drm/msm/dpu: include SSPP allocation state into the dumped state drivers/gpu/drm/drm_atomic_helper.c | 110 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 59 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 26 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 6 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 671 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 29 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 130 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 36 ++ include/drm/drm_atomic_helper.h | 7 + 10 files changed, 924 insertions(+), 152 deletions(-) -- 2.39.2
[Freedreno] [PATCH v3 11/12] drm/msm/dpu: allow sharing of blending stages
It is possible to slightly bend the limitations of the HW blender. If
two rectangles are contiguous (like two rectangles of a single plane)
they can be blended using a single LM blending stage, allowing one to
blend more planes via a single LM.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 --
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 37 ++-
2 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index df4c2e503fa5..4b5b2b7ed494 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -456,6 +456,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
uint32_t lm_idx;
bool bg_alpha_enable = false;
+ unsigned int stage_indices[DPU_STAGE_MAX] = {};
DECLARE_BITMAP(fetch_active, SSPP_MAX);
memset(fetch_active, 0, sizeof(fetch_active));
@@ -480,7 +481,9 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
mixer, cstate->num_mixers,
pstate->stage,
format, fb ? fb->modifier : 0,
- >pipe, 0, stage_cfg);
+ >pipe,
+ stage_indices[pstate->stage]++,
+ stage_cfg);
if (pstate->r_pipe.sspp) {
set_bit(pstate->r_pipe.sspp->idx, fetch_active);
@@ -488,7 +491,9 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc
*crtc,
mixer, cstate->num_mixers,
pstate->stage,
format, fb ? fb->modifier :
0,
- >r_pipe, 1,
stage_cfg);
+ >r_pipe,
+
stage_indices[pstate->stage]++,
+ stage_cfg);
}
/* blend config update */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 61afd1cf033d..e7a157feab22 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -809,13 +809,6 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane
*plane,
if (!new_plane_state->visible)
return 0;
- 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;
- }
-
fb_rect.x2 = new_plane_state->fb->width;
fb_rect.y2 = new_plane_state->fb->height;
@@ -952,6 +945,18 @@ static int dpu_plane_try_multirect(struct dpu_plane_state
*pstate,
prev_pipe->multirect_index = DPU_SSPP_RECT_0;
prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+ if (pipe_cfg->dst_rect.y1 == prev_pipe_cfg->dst_rect.y1 &&
+ pipe_cfg->dst_rect.y2 == prev_pipe_cfg->dst_rect.y2 &&
+ pipe_cfg->dst_rect.x1 == prev_pipe_cfg->dst_rect.x2) {
+ pstate->stage = prev_pstate->stage;
+ } else if (pipe_cfg->dst_rect.y1 == prev_pipe_cfg->dst_rect.y1
&&
+ pipe_cfg->dst_rect.y2 == prev_pipe_cfg->dst_rect.y2
&&
+ pipe_cfg->dst_rect.x2 == prev_pipe_cfg->dst_rect.x1)
{
+ pstate->stage = prev_pstate->stage;
+ pipe->multirect_index = DPU_SSPP_RECT_0;
+ prev_pipe->multirect_index = DPU_SSPP_RECT_1;
+ }
+
return true;
}
@@ -1054,6 +1059,13 @@ static int dpu_plane_atomic_check(struct drm_plane
*plane,
if (!new_plane_state->visible)
return 0;
+ 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;
+ }
+
pipe->multirect_index = DPU_SSPP_RECT_SOLO;
pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
@@ -1189,6 +1201,11 @@ static int dpu_plane_virtual_assign_resources(struct
drm_crtc *crtc,
max_linewidth = dpu_kms->catalog->caps->max_linewidth;
+ if (prev_pstate)
+
[Freedreno] [PATCH v3 12/12] drm/msm/dpu: include SSPP allocation state into the dumped state
Make dpu_rm_print_state() also output the SSPP allocation state. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 1b0166bc9bee..e7c142bebab1 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -773,4 +773,12 @@ void dpu_rm_print_state(struct drm_printer *p, else drm_puts(p, " -,"); drm_puts(p, "\n"); + + drm_puts(p, "sspp:"); + for (i = 0; i < ARRAY_SIZE(global_state->sspp_to_crtc_id); i++) + if (rm->hw_sspp[i]) + drm_printf(p, " %d,", global_state->sspp_to_crtc_id[i]); + else + drm_puts(p, " -,"); + drm_puts(p, "\n"); } -- 2.39.2
[Freedreno] [PATCH v3 08/12] drm/msm/dpu: allow using two SSPP blocks for a single plane
Virtual wide planes give high amount of flexibility, but it is not
always enough:
In parallel multirect case only the half of the usual width is supported
for tiled formats. Thus the whole width of two tiled multirect
rectangles can not be greater than max_linewidth, which is not enough
for some platforms/compositors.
Another example is as simple as wide YUV plane. YUV planes can not use
multirect, so currently they are limited to max_linewidth too.
Now that the planes are fully virtualized, add support for allocating
two SSPP blocks to drive a single DRM plane. This fixes both mentioned
cases and allows all planes to go up to 2*max_linewidth (at the cost of
making some of the planes unavailable to the user).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 170 --
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 8 +
2 files changed, 130 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 2ac6e1074c62..9ffbcd91661a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -867,6 +867,28 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane
*plane,
return 0;
}
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+ struct dpu_sw_pipe_cfg
*pipe_cfg,
+ const struct dpu_format *fmt,
+ uint32_t max_linewidth)
+{
+ if (drm_rect_width(_cfg->src_rect) !=
drm_rect_width(_cfg->dst_rect) ||
+ drm_rect_height(_cfg->src_rect) !=
drm_rect_height(_cfg->dst_rect))
+ return false;
+
+ if (pipe_cfg->rotation & DRM_MODE_ROTATE_90)
+ return false;
+
+ if (DPU_FORMAT_IS_YUV(fmt))
+ return false;
+
+ if (DPU_FORMAT_IS_UBWC(fmt) &&
+ drm_rect_width(_cfg->src_rect) > max_linewidth / 2)
+ return false;
+
+ return true;
+}
+
static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
struct drm_atomic_state *state)
{
@@ -880,7 +902,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
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;
- uint32_t max_linewidth;
uint32_t supported_rotations;
const struct dpu_sspp_cfg *pipe_hw_caps;
const struct dpu_sspp_sub_blks *sblk;
@@ -895,15 +916,8 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
if (ret)
return ret;
- 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;
-
fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
- max_linewidth = pdpu->catalog->caps->max_linewidth;
-
supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
@@ -918,40 +932,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
return ret;
if (drm_rect_width(_pipe_cfg->src_rect) != 0) {
- /*
-* In parallel multirect case only the half of the usual width
-* is supported for tiled formats. If we are here, we know that
-* full width is more than max_linewidth, thus each rect is
-* wider than allowed.
-*/
- if (DPU_FORMAT_IS_UBWC(fmt)) {
- DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
line:%u, tiled format\n",
- DRM_RECT_ARG(_cfg->src_rect),
max_linewidth);
- return -E2BIG;
- }
-
- if (drm_rect_width(_cfg->src_rect) !=
drm_rect_width(_cfg->dst_rect) ||
- drm_rect_height(_cfg->src_rect) !=
drm_rect_height(_cfg->dst_rect) ||
- (!test_bit(DPU_SSPP_SMART_DMA_V1,
>sspp->cap->features) &&
-!test_bit(DPU_SSPP_SMART_DMA_V2,
>sspp->cap->features)) ||
- pipe_cfg->rotation & DRM_MODE_ROTATE_90 ||
- DPU_FORMAT_IS_YUV(fmt)) {
- DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
line:%u, can't use split source\n",
- DRM_RECT_ARG(_cfg->src_rect),
max_linewidth);
- return -E2BIG;
- }
-
- /*
-* Use multirect for wide plane. We do not support dynamic
-* assignment of SSPPs, so we know the configuration.
-*/
-
[Freedreno] [PATCH v3 09/12] drm/msm/dpu: allow sharing SSPP between planes
Since SmartDMA planes provide two rectangles, it is possible to use them
to drive two different DRM planes, first plane getting the rect_0,
another one using rect_1 of the same SSPP. The sharing algorithm is
pretty simple, it requires that each of the planes can be driven by the
single rectangle and only consequetive planes are considered.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 128 +++---
1 file changed, 112 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 9ffbcd91661a..61afd1cf033d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -867,10 +867,9 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane
*plane,
return 0;
}
-static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
- struct dpu_sw_pipe_cfg
*pipe_cfg,
- const struct dpu_format *fmt,
- uint32_t max_linewidth)
+static int dpu_plane_is_multirect_capable(struct dpu_sw_pipe *pipe,
+ struct dpu_sw_pipe_cfg *pipe_cfg,
+ const struct dpu_format *fmt)
{
if (drm_rect_width(_cfg->src_rect) !=
drm_rect_width(_cfg->dst_rect) ||
drm_rect_height(_cfg->src_rect) !=
drm_rect_height(_cfg->dst_rect))
@@ -882,6 +881,13 @@ static int dpu_plane_is_multirect_parallel_capable(struct
dpu_sw_pipe *pipe,
if (DPU_FORMAT_IS_YUV(fmt))
return false;
+ return true;
+}
+
+static int dpu_plane_is_parallel_capable(struct dpu_sw_pipe_cfg *pipe_cfg,
+const struct dpu_format *fmt,
+uint32_t max_linewidth)
+{
if (DPU_FORMAT_IS_UBWC(fmt) &&
drm_rect_width(_cfg->src_rect) > max_linewidth / 2)
return false;
@@ -889,6 +895,82 @@ static int dpu_plane_is_multirect_parallel_capable(struct
dpu_sw_pipe *pipe,
return true;
}
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+ struct dpu_sw_pipe_cfg
*pipe_cfg,
+ const struct dpu_format *fmt,
+ uint32_t max_linewidth)
+{
+ return dpu_plane_is_multirect_capable(pipe, pipe_cfg, fmt) &&
+ dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth);
+}
+
+
+static int dpu_plane_try_multirect(struct dpu_plane_state *pstate,
+ struct dpu_plane_state *prev_pstate,
+ const struct dpu_format *fmt,
+ uint32_t max_linewidth)
+{
+ struct dpu_sw_pipe *pipe = >pipe;
+ struct dpu_sw_pipe *r_pipe = >r_pipe;
+ struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
+ struct dpu_sw_pipe *prev_pipe = _pstate->pipe;
+ struct dpu_sw_pipe_cfg *prev_pipe_cfg = _pstate->pipe_cfg;
+ const struct dpu_format *prev_fmt =
+ to_dpu_format(msm_framebuffer_format(prev_pstate->base.fb));
+ u16 max_tile_height = 1;
+
+ if (prev_pstate->r_pipe.sspp != NULL ||
+ prev_pipe->multirect_mode != DPU_SSPP_MULTIRECT_NONE)
+ return false;
+
+ if (!dpu_plane_is_multirect_capable(pipe, pipe_cfg, fmt) ||
+ !dpu_plane_is_multirect_capable(prev_pipe, prev_pipe_cfg, prev_fmt)
||
+ !(test_bit(DPU_SSPP_SMART_DMA_V1, _pipe->sspp->cap->features)
||
+ test_bit(DPU_SSPP_SMART_DMA_V2, _pipe->sspp->cap->features)))
+ return false;
+
+ if (DPU_FORMAT_IS_UBWC(fmt))
+ max_tile_height = max(max_tile_height, fmt->tile_height);
+
+ if (DPU_FORMAT_IS_UBWC(prev_fmt))
+ max_tile_height = max(max_tile_height, prev_fmt->tile_height);
+
+ r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
+ r_pipe->sspp = NULL;
+
+ if (dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth) &&
+ dpu_plane_is_parallel_capable(prev_pipe_cfg, prev_fmt,
max_linewidth) &&
+ (pipe_cfg->dst_rect.x1 >= prev_pipe_cfg->dst_rect.x2 ||
+prev_pipe_cfg->dst_rect.x1 >= pipe_cfg->dst_rect.x2)) {
+ pipe->sspp = prev_pipe->sspp;
+
+ pipe->multirect_index = DPU_SSPP_RECT_1;
+ pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+
+ prev_pipe->multirect_index = DPU_SSPP_RECT_0;
+ prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
+
+ return true;
+ }
+
+ if (pipe_cfg->dst_rect.y1 >= prev_pipe_cfg->dst_rect.y2 + 2 *
max_tile_height ||
+
[Freedreno] [PATCH v3 10/12] drm/msm/dpu: create additional virtual planes
Since we have enabled sharing of SSPP blocks between two planes, it is
now possible to use twice as much planes as there are hardware SSPP
blocks. Create additional overlay planes.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 5106abe366a3..a6cd1346b298 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -792,6 +792,18 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
primary_planes[primary_planes_idx++] = plane;
}
+ if (dpu_use_virtual_planes) {
+ for (i = 0; i < catalog->sspp_count; i++) {
+ plane = dpu_plane_init_virtual(dev,
DRM_PLANE_TYPE_OVERLAY,
+ (1UL << max_crtc_count)
- 1);
+ if (IS_ERR(plane)) {
+ DPU_ERROR("dpu_plane_init failed\n");
+ ret = PTR_ERR(plane);
+ return ret;
+ }
+ }
+ }
+
max_crtc_count = min(max_crtc_count, primary_planes_idx);
/* Create one CRTC per encoder */
--
2.39.2
[Freedreno] [PATCH v3 07/12] drm/msm/dpu: add support for virtual planes
Only several SSPP blocks support such features as YUV output or scaling,
thus different DRM planes have different features. Properly utilizing
all planes requires the attention of the compositor, who should
prefer simpler planes to YUV-supporting ones. Otherwise it is very easy
to end up in a situation when all featureful planes are already
allocated for simple windows, leaving no spare plane for YUV playback.
To solve this problem make all planes virtual. Each plane is registered
as if it supports all possible features, but then at the runtime during
the atomic_check phase the driver selects backing SSPP block for each
plane.
Note, this does not provide support for using two different SSPP blocks
for a single plane or using two rectangles of an SSPP to drive two
planes. Each plane still gets its own SSPP and can utilize either a solo
rectangle or both multirect rectangles depending on the resolution.
Note #2: By default support for virtual planes is turned off and the
driver still uses old code path with preallocated SSPP block for each
plane. To enable virtual planes, pass 'msm.dpu_use_virtual_planes=1'
kernel parameter.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 50 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 228 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 19 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 74 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h| 28 +++
7 files changed, 384 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 8ce7586e2ddf..df4c2e503fa5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1179,6 +1179,49 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state
*cstate)
return false;
}
+static int dpu_crtc_reassign_planes(struct drm_crtc *crtc, struct
drm_crtc_state *crtc_state)
+{
+ int total_planes = crtc->dev->mode_config.num_total_plane;
+ struct drm_atomic_state *state = crtc_state->state;
+ struct dpu_global_state *global_state;
+ struct drm_plane_state **states;
+ struct drm_plane *plane;
+ int ret;
+
+ global_state = dpu_kms_get_global_state(crtc_state->state);
+ if (IS_ERR(global_state))
+ return PTR_ERR(global_state);
+
+ dpu_rm_release_all_sspp(global_state, crtc);
+
+ if (!crtc_state->enable)
+ return 0;
+
+ states = kcalloc(total_planes, sizeof(*states), GFP_KERNEL);
+ if (!states)
+ return -ENOMEM;
+
+ drm_atomic_crtc_state_for_each_plane(plane, crtc_state) {
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_plane_state(state, plane);
+
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto done;
+ }
+
+ states[plane_state->normalized_zpos] = plane_state;
+ }
+
+ ret = dpu_assign_plane_resources(global_state, state, crtc, states,
total_planes);
+
+done:
+ kfree(states);
+ return ret;
+
+ return 0;
+}
+
static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -1194,6 +1237,13 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
+ if (dpu_use_virtual_planes &&
+ (crtc_state->planes_changed || crtc_state->zpos_changed)) {
+ rc = dpu_crtc_reassign_planes(crtc, crtc_state);
+ if (rc < 0)
+ return rc;
+ }
+
if (!crtc_state->enable ||
!drm_atomic_crtc_effectively_active(crtc_state)) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip
atomic_check\n",
crtc->base.id, crtc_state->enable,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 172b64dc60e6..5106abe366a3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -51,6 +51,9 @@
#define DPU_DEBUGFS_DIR "msm_dpu"
#define DPU_DEBUGFS_HWMASKNAME "hw_log_mask"
+bool dpu_use_virtual_planes = false;
+module_param(dpu_use_virtual_planes, bool, 0);
+
static int dpu_kms_hw_init(struct msm_kms *kms);
static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms);
@@ -772,8 +775,11 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
type, catalog->sspp[i].features,
catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR));
- plane = dpu_plane_init(dev, catalog->sspp[i].id, type,
- (1UL << max_crtc_count) - 1);
+ if
[Freedreno] [PATCH v3 06/12] drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe()
Move a call to dpu_plane_check_inline_rotation() to the
dpu_plane_atomic_check_pipe() function, so that the rot90 constraints
are checked for both pipes. Also move rotation field from struct
dpu_plane_state to struct dpu_sw_pipe_cfg.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 55 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 2 -
3 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index cbf4f95ff0fd..21b0c5a13ba8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -157,10 +157,12 @@ struct dpu_hw_pixel_ext {
* @src_rect: src ROI, caller takes into account the different operations
* such as decimation, flip etc to program this field
* @dest_rect: destination ROI.
+ * @rotation: simplified drm rotation hint
*/
struct dpu_sw_pipe_cfg {
struct drm_rect src_rect;
struct drm_rect dst_rect;
+ unsigned int rotation;
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 4b1fbe9dbb3f..4cf69f93b44f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -527,8 +527,7 @@ static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct
dpu_sw_pipe *pipe,
static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
const struct dpu_format *fmt, bool color_fill,
- struct dpu_sw_pipe_cfg *pipe_cfg,
- unsigned int rotation)
+ struct dpu_sw_pipe_cfg *pipe_cfg)
{
struct dpu_hw_sspp *pipe_hw = pipe->sspp;
const struct drm_format_info *info =
drm_format_info(fmt->base.pixel_format);
@@ -551,7 +550,7 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe
*pipe,
dst_height,
_cfg, fmt,
info->hsub, info->vsub,
- rotation);
+ pipe_cfg->rotation);
/* configure pixel extension based on scalar config */
_dpu_plane_setup_pixel_ext(_cfg, _ext,
@@ -603,7 +602,7 @@ static void _dpu_plane_color_fill_pipe(struct
dpu_plane_state *pstate,
if (pipe->sspp->ops.setup_rects)
pipe->sspp->ops.setup_rects(pipe, _cfg);
- _dpu_plane_setup_scaler(pipe, fmt, true, _cfg, pstate->rotation);
+ _dpu_plane_setup_scaler(pipe, fmt, true, _cfg);
}
/**
@@ -703,12 +702,17 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane,
}
static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
- const struct dpu_sspp_sub_blks
*sblk,
- struct drm_rect src, const
struct dpu_format *fmt)
+ struct dpu_sw_pipe *pipe,
+ struct drm_rect src,
+ const struct dpu_format *fmt)
{
+ const struct dpu_sspp_sub_blks *sblk = pipe->sspp->cap->sblk;
size_t num_formats;
const u32 *supported_formats;
+ if (!test_bit(DPU_SSPP_INLINE_ROTATION, >sspp->cap->features))
+ return -EINVAL;
+
if (!sblk->rotation_cfg) {
DPU_ERROR("invalid rotation cfg\n");
return -EINVAL;
@@ -736,6 +740,7 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
const struct dpu_format *fmt)
{
uint32_t min_src_size;
+ int ret;
min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -774,6 +779,12 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
return -EINVAL;
}
+ if (pipe_cfg->rotation & DRM_MODE_ROTATE_90) {
+ ret = dpu_plane_check_inline_rotation(pdpu, pipe,
pipe_cfg->src_rect, fmt);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
@@ -870,7 +881,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
uint32_t max_linewidth;
- unsigned int rotation;
uint32_t supported_rotations;
const struct dpu_sspp_cfg *pipe_hw_caps;
const struct dpu_sspp_sub_blks *sblk;
@@ -894,6 +904,15 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
max_linewidth = pdpu->catalog->caps->max_linewidth;
+ supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
+
+ if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
+ supported_rotations |= DRM_MODE_ROTATE_90;
+
+ pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
+
[Freedreno] [PATCH v3 04/12] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check
In preparation for virtualized planes support, move pstate->pipe
initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In
case of virtual planes the plane's pipe will not be known up to the
point of atomic_check() callback.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 26 +++
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 67f9c2a62a17..3a75c474c4cd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -785,6 +785,7 @@ 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_kms *dpu_kms = _dpu_plane_get_kms(plane);
struct dpu_sw_pipe *pipe = >pipe;
struct dpu_sw_pipe *r_pipe = >r_pipe;
const struct drm_crtc_state *crtc_state = NULL;
@@ -795,13 +796,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
uint32_t max_linewidth;
unsigned int rotation;
uint32_t supported_rotations;
- const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap;
- const struct dpu_sspp_sub_blks *sblk = pstate->pipe.sspp->cap->sblk;
+ const struct dpu_sspp_cfg *pipe_hw_caps;
+ const struct dpu_sspp_sub_blks *sblk;
if (new_plane_state->crtc)
crtc_state = drm_atomic_get_new_crtc_state(state,
new_plane_state->crtc);
+ pipe->sspp = dpu_rm_get_sspp(_kms->rm, pdpu->pipe);
+ r_pipe->sspp = NULL;
+
+ if (!pipe->sspp)
+ return -EINVAL;
+
+ pipe_hw_caps = pipe->sspp->cap;
+ sblk = pipe->sspp->cap->sblk;
+
min_scale = FRAC_16_16(1, sblk->maxupscale);
ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
min_scale,
@@ -818,7 +828,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
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) {
@@ -1302,7 +1311,6 @@ static void dpu_plane_reset(struct drm_plane *plane)
{
struct dpu_plane *pdpu;
struct dpu_plane_state *pstate;
- struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
if (!plane) {
DPU_ERROR("invalid plane\n");
@@ -1324,16 +1332,6 @@ static void dpu_plane_reset(struct drm_plane *plane)
return;
}
- /*
-* Set the SSPP here until we have proper virtualized DPU planes.
-* This is the place where the state is allocated, so fill it fully.
-*/
- pstate->pipe.sspp = dpu_rm_get_sspp(_kms->rm, pdpu->pipe);
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
- pstate->r_pipe.sspp = NULL;
-
__drm_atomic_helper_plane_reset(plane, >base);
}
--
2.39.2
[Freedreno] [PATCH v3 02/12] drm/msm/dpu: add current resource allocation to dumped state
Provide atomic_print_state callback to the DPU's private object. This
way the debugfs/dri/0/state will also include RM's internal state.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 48 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 8 +
4 files changed, 62 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ee84160592ce..172b64dc60e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -362,6 +362,7 @@ static void dpu_kms_global_destroy_state(struct
drm_private_obj *obj,
static const struct drm_private_state_funcs dpu_kms_global_state_funcs = {
.atomic_duplicate_state = dpu_kms_global_duplicate_state,
.atomic_destroy_state = dpu_kms_global_destroy_state,
+ .atomic_print_state = dpu_rm_print_state,
};
static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
@@ -375,6 +376,9 @@ static int dpu_kms_global_obj_init(struct dpu_kms *dpu_kms)
drm_atomic_private_obj_init(dpu_kms->dev, _kms->global_state,
>base,
_kms_global_state_funcs);
+
+ state->rm = _kms->rm;
+
return 0;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index ed549f0f7c65..dd2be279b366 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -130,6 +130,8 @@ struct vsync_info {
struct dpu_global_state {
struct drm_private_state base;
+ struct dpu_rm *rm;
+
uint32_t pingpong_to_enc_id[PINGPONG_MAX - PINGPONG_0];
uint32_t mixer_to_enc_id[LM_MAX - LM_0];
uint32_t ctl_to_enc_id[CTL_MAX - CTL_0];
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643c71a..5e3442fb8678 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -652,3 +652,51 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
return num_blks;
}
+
+void dpu_rm_print_state(struct drm_printer *p,
+ const struct drm_private_state *state)
+{
+ const struct dpu_global_state *global_state =
to_dpu_global_state(state);
+ const struct dpu_rm *rm = global_state->rm;
+ int i;
+
+ drm_puts(p, "pingpong:");
+ for (i = 0; i < ARRAY_SIZE(global_state->pingpong_to_enc_id); i++)
+ if (rm->pingpong_blks[i])
+ drm_printf(p, " %d,",
global_state->pingpong_to_enc_id[i]);
+ else
+ drm_puts(p, " -,");
+ drm_puts(p, "\n");
+
+ drm_puts(p, "mixer:");
+ for (i = 0; i < ARRAY_SIZE(global_state->mixer_to_enc_id); i++)
+ if (rm->mixer_blks[i])
+ drm_printf(p, " %d,", global_state->mixer_to_enc_id[i]);
+ else
+ drm_puts(p, " -,");
+ drm_puts(p, "\n");
+
+ drm_puts(p, "ctl:");
+ for (i = 0; i < ARRAY_SIZE(global_state->ctl_to_enc_id); i++)
+ if (rm->ctl_blks[i])
+ drm_printf(p, " %d,", global_state->ctl_to_enc_id[i]);
+ else
+ drm_puts(p, " -,");
+ drm_puts(p, "\n");
+
+ drm_puts(p, "dspp:");
+ for (i = 0; i < ARRAY_SIZE(global_state->dspp_to_enc_id); i++)
+ if (rm->dspp_blks[i])
+ drm_printf(p, " %d,", global_state->dspp_to_enc_id[i]);
+ else
+ drm_puts(p, " -,");
+ drm_puts(p, "\n");
+
+ drm_puts(p, "dsc:");
+ for (i = 0; i < ARRAY_SIZE(global_state->dsc_to_enc_id); i++)
+ if (rm->dsc_blks[i])
+ drm_printf(p, " %d,", global_state->dsc_to_enc_id[i]);
+ else
+ drm_puts(p, " -,");
+ drm_puts(p, "\n");
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 2b551566cbf4..913baca81a42 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -92,6 +92,14 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
struct dpu_global_state *global_state, uint32_t enc_id,
enum dpu_hw_blk_type type, struct dpu_hw_blk **blks, int blks_size);
+/**
+ * dpu_rm_print_state - output the RM private state
+ * @p: DRM printer
+ * @state: private object state
+ */
+void dpu_rm_print_state(struct drm_printer *p,
+ const struct drm_private_state *state);
+
/**
* dpu_rm_get_intf - Return a struct dpu_hw_intf instance given it's index.
* @rm: DPU Resource Manager handle
--
2.39.2
[Freedreno] [PATCH v3 05/12] drm/msm/dpu: split dpu_plane_atomic_check()
Split dpu_plane_atomic_check() function into two pieces:
dpu_plane_atomic_check_nopipe() performing generic checks on the pstate,
without touching the associated pipe,
and
dpu_plane_atomic_check_pipes(), which takes into account used pipes.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 178 ++
1 file changed, 112 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 3a75c474c4cd..4b1fbe9dbb3f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -777,46 +777,20 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
return 0;
}
-static int dpu_plane_atomic_check(struct drm_plane *plane,
- struct drm_atomic_state *state)
+static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
+struct drm_plane_state
*new_plane_state,
+const struct drm_crtc_state
*crtc_state)
{
- struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
-
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_kms *dpu_kms = _dpu_plane_get_kms(plane);
- 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;
- uint32_t supported_rotations;
- const struct dpu_sspp_cfg *pipe_hw_caps;
- const struct dpu_sspp_sub_blks *sblk;
+ int ret = 0;
- if (new_plane_state->crtc)
- crtc_state = drm_atomic_get_new_crtc_state(state,
-
new_plane_state->crtc);
-
- pipe->sspp = dpu_rm_get_sspp(_kms->rm, pdpu->pipe);
- r_pipe->sspp = NULL;
-
- if (!pipe->sspp)
- return -EINVAL;
-
- pipe_hw_caps = pipe->sspp->cap;
- sblk = pipe->sspp->cap->sblk;
-
- min_scale = FRAC_16_16(1, sblk->maxupscale);
- ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
- min_scale,
- sblk->maxdwnscale << 16,
- true, true);
+ ret = drm_atomic_helper_check_plane_noscale(new_plane_state, crtc_state,
+ true, true);
if (ret) {
DPU_DEBUG_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
return ret;
@@ -824,11 +798,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
- 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;
-
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",
@@ -847,8 +816,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -E2BIG;
}
- fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
-
max_linewidth = pdpu->catalog->caps->max_linewidth;
/* state->src is 16.16, src_rect is not */
@@ -861,6 +828,77 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
new_plane_state->rotation);
if (drm_rect_width(_cfg->src_rect) > max_linewidth) {
+ if (drm_rect_width(_cfg->src_rect) > 2 * max_linewidth) {
+ DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
line:%u\n",
+ DRM_RECT_ARG(_cfg->src_rect),
max_linewidth);
+ return -E2BIG;
+ }
+
+ *r_pipe_cfg = *pipe_cfg;
+ pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 +
pipe_cfg->src_rect.x2) >> 1;
+ pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 +
pipe_cfg->dst_rect.x2) >> 1;
+ r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
+ r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
+ } else {
+ memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg));
+ }
+
+ drm_rect_rotate_inv(_cfg->src_rect,
+
[Freedreno] [PATCH v3 03/12] drm/msm/dpu: take plane rotation into account for wide planes
Take into account the plane rotation and flipping when calculating src
positions for the wide plane parts.
This is not an issue yet, because rotation is only supported for the
UBWC planes and wide UBWC planes are rejected anyway because in parallel
multirect case only the half of the usual width is supported for tiled
formats. However it's better to fix this now rather than stumbling upon
it later.
Fixes: 80e8ae3b38ab ("drm/msm/dpu: add support for wide planes")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 27 ++-
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..67f9c2a62a17 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -827,16 +827,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}
- pipe_cfg->src_rect = new_plane_state->src;
-
- /* state->src is 16.16, src_rect is not */
- pipe_cfg->src_rect.x1 >>= 16;
- pipe_cfg->src_rect.x2 >>= 16;
- pipe_cfg->src_rect.y1 >>= 16;
- pipe_cfg->src_rect.y2 >>= 16;
-
- pipe_cfg->dst_rect = new_plane_state->dst;
-
fb_rect.x2 = new_plane_state->fb->width;
fb_rect.y2 = new_plane_state->fb->height;
@@ -852,6 +842,15 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
max_linewidth = pdpu->catalog->caps->max_linewidth;
+ /* state->src is 16.16, src_rect is not */
+ drm_rect_fp_to_int(_cfg->src_rect, _plane_state->src);
+
+ pipe_cfg->dst_rect = new_plane_state->dst;
+
+ drm_rect_rotate(_cfg->src_rect,
+ new_plane_state->fb->width, new_plane_state->fb->height,
+ new_plane_state->rotation);
+
if (drm_rect_width(_cfg->src_rect) > max_linewidth) {
/*
* In parallel multirect case only the half of the usual width
@@ -899,6 +898,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
}
+ drm_rect_rotate_inv(_cfg->src_rect,
+ new_plane_state->fb->width,
new_plane_state->fb->height,
+ new_plane_state->rotation);
+ if (r_pipe->sspp)
+ drm_rect_rotate_inv(_pipe_cfg->src_rect,
+ new_plane_state->fb->width,
new_plane_state->fb->height,
+ new_plane_state->rotation);
+
ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt);
if (ret)
return ret;
--
2.39.2
[Freedreno] [PATCH v3 01/12] drm/atomic-helper: split not-scaling part of drm_atomic_helper_check_plane_state
The helper drm_atomic_helper_check_plane_state() runs several checks on
plane src and dst rectangles, including the check whether required
scaling fits into the required margins. The msm driver would benefit
from having a function that does all these checks except the scaling
one. Split them into a new helper called
drm_atomic_helper_check_plane_noscale().
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_atomic_helper.c | 110 ++--
include/drm/drm_atomic_helper.h | 7 ++
2 files changed, 96 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 292e38eb6218..2d7dd66181c9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct
drm_encoder *encoder,
EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
/**
- * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
* @plane_state: plane state to check
* @crtc_state: CRTC state to check
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
* @can_position: is it legal to position the plane such that it
*doesn't cover the entire CRTC? This will generally
*only be false for primary planes.
@@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
* RETURNS:
* Zero if update appears valid, error code on failure
*/
-int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
- const struct drm_crtc_state *crtc_state,
- int min_scale,
- int max_scale,
- bool can_position,
- bool can_update_disabled)
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state
*crtc_state,
+ bool can_position,
+ bool can_update_disabled)
{
struct drm_framebuffer *fb = plane_state->fb;
struct drm_rect *src = _state->src;
struct drm_rect *dst = _state->dst;
unsigned int rotation = plane_state->rotation;
struct drm_rect clip = {};
- int hscale, vscale;
WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
@@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
- /* Check scaling */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (hscale < 0 || vscale < 0) {
- drm_dbg_kms(plane_state->plane->dev,
- "Invalid scaling of plane\n");
- drm_rect_debug_print("src: ", _state->src, true);
- drm_rect_debug_print("dst: ", _state->dst, false);
- return -ERANGE;
- }
-
if (crtc_state->enable)
drm_mode_get_hv_timing(_state->mode, , );
@@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct
drm_plane_state *plane_state,
return 0;
}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
+
+/**
+ * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
+ * @plane_state: plane state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ *
+ * Checks that a desired plane scale fits into the min_scale..max_scale
+ * boundaries.
+ * Drivers that provide their own plane handling rather than helper-provided
+ * implementations may still wish to call this function to avoid duplication of
+ * error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+ int min_scale,
+ int max_scale)
+{
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect src;
+ struct drm_rect dst;
+ int hscale, vscale;
+
+ if (!plane_state->visible)
+ return 0;
+
+ src = drm_plane_state_src(plane_state);
+ dst = drm_plane_state_dest(plane_state);
+
+ drm_rect_rotate(, fb->width << 16, fb->height << 16,
plane_state->rotation);
+
+ hscale = drm_rect_calc_hscale(, , min_scale, max_scale);
+ vscale = drm_rect_calc_vscale(, , min_scale, max_scale);
+ if (hscale < 0 || vscale < 0)
[Freedreno] [PATCH] drm/msm/adreno: Add support for SM7150 SoC machine
SM7150 has 5 power levels which correspond to 5 speed-bin values: 0,
128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up
to zero decimal places.
The vendor's FW GMU is called a618_gmu.bin. And also a618 on SM7150 uses
a615 zapfw.
Add this as machine = "qcom,sm7150", because speed-bin values are
different from atoll (sc7180/sm7125).
Signed-off-by: Danila Tikhonov
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fa527935ffd4..64ef9813e9ae 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -293,6 +293,28 @@ static const struct adreno_info gpulist[] = {
{ 157, 3 },
{ 127, 4 },
),
+ }, {
+ .machine = "qcom,sm7150",
+ .chip_ids = ADRENO_CHIP_IDS(0x06010800),
+ .family = ADRENO_6XX_GEN1,
+ .revn = 618,
+ .fw = {
+ [ADRENO_FW_SQE] = "a630_sqe.fw",
+ [ADRENO_FW_GMU] = "a618_gmu.bin",
+ },
+ .gmem = SZ_512K,
+ .inactive_period = DRM_MSM_INACTIVE_PERIOD,
+ .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
+ .init = a6xx_gpu_init,
+ .zapfw = "a615_zap.mdt",
+ .hwcg = a615_hwcg,
+ .speedbins = ADRENO_SPEEDBINS(
+ { 0, 0 },
+ { 128, 1 },
+ { 146, 2 },
+ { 167, 3 },
+ { 172, 4 },
+ ),
}, {
.chip_ids = ADRENO_CHIP_IDS(0x06010800),
.family = ADRENO_6XX_GEN1,
@@ -507,6 +529,10 @@ MODULE_FIRMWARE("qcom/a530_zap.b00");
MODULE_FIRMWARE("qcom/a530_zap.b01");
MODULE_FIRMWARE("qcom/a530_zap.b02");
MODULE_FIRMWARE("qcom/a540_gpmu.fw2");
+MODULE_FIRMWARE("qcom/a615_zap.mbt");
+MODULE_FIRMWARE("qcom/a615_zap.b00");
+MODULE_FIRMWARE("qcom/a615_zap.b01");
+MODULE_FIRMWARE("qcom/a615_zap.b02");
MODULE_FIRMWARE("qcom/a619_gmu.bin");
MODULE_FIRMWARE("qcom/a630_sqe.fw");
MODULE_FIRMWARE("qcom/a630_gmu.bin");
--
2.41.0
Re: [Freedreno] [PATCH] drm/msm/adreno: Add support for SM7150 SoC machine
On 13.09.2023 21:19, Danila Tikhonov wrote:
> SM7150 has 5 power levels which correspond to 5 speed-bin values: 0,
> 128, 146, 167, 172. Speed-bin value is calulated as FMAX/4.8MHz round up
> to zero decimal places.
>
> The vendor's FW GMU is called a618_gmu.bin. And also a618 on SM7150 uses
> a615 zapfw.
Interesting.
GMU fw comes from Qualcomm and should not(?) have any
vendor modifications, btw.
Can you try loading the upstream a630_gmu.bin or a619_gmu.bin
from linux-firmware and seeing if anything changes?
> + }, {
> + .machine = "qcom,sm7150",
> + .chip_ids = ADRENO_CHIP_IDS(0x06010800),
> + .family = ADRENO_6XX_GEN1,
> + .revn = 618,
I'm not sure what Rob's stance on using revn is for values
that have already been used before..
Konrad
> + .fw = {
> + [ADRENO_FW_SQE] = "a630_sqe.fw",
> + [ADRENO_FW_GMU] = "a618_gmu.bin",
> + },
> + .gmem = SZ_512K,
> + .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> + .quirks = ADRENO_QUIRK_HAS_CACHED_COHERENT,
> + .init = a6xx_gpu_init,
> + .zapfw = "a615_zap.mdt",
.mbn?
> + .hwcg = a615_hwcg,
> + .speedbins = ADRENO_SPEEDBINS(
> + { 0, 0 },
> + { 128, 1 },
> + { 146, 2 },
> + { 167, 3 },
> + { 172, 4 },
> + ),
> }, {
> .chip_ids = ADRENO_CHIP_IDS(0x06010800),
> .family = ADRENO_6XX_GEN1,
> @@ -507,6 +529,10 @@ MODULE_FIRMWARE("qcom/a530_zap.b00");
> MODULE_FIRMWARE("qcom/a530_zap.b01");
> MODULE_FIRMWARE("qcom/a530_zap.b02");
> MODULE_FIRMWARE("qcom/a540_gpmu.fw2");
> +MODULE_FIRMWARE("qcom/a615_zap.mbt");
> +MODULE_FIRMWARE("qcom/a615_zap.b00");
> +MODULE_FIRMWARE("qcom/a615_zap.b01");
> +MODULE_FIRMWARE("qcom/a615_zap.b02");
and here too?
Konrad
Re: [Freedreno] [PATCH v4 6/6] drm/drm-file: Show finer-grained BO sizes in drm_show_memory_stats
On Wed, 13 Sep 2023 09:46:45 -0700 Rob Clark wrote: > On Wed, Sep 13, 2023 at 12:36 AM Boris Brezillon > wrote: > > > > On Tue, 12 Sep 2023 19:14:35 -0700 > > Rob Clark wrote: > > > > > On Tue, Sep 12, 2023 at 6:46 PM Rob Clark wrote: > > > > > > > > On Tue, Sep 12, 2023 at 2:32 AM Boris Brezillon > > > > wrote: > > > > > > > > > > On Tue, 12 Sep 2023 09:37:00 +0100 > > > > > Adrián Larumbe wrote: > > > > > > > > > > > The current implementation will try to pick the highest available > > > > > > size > > > > > > display unit as soon as the BO size exceeds that of the previous > > > > > > multiplier. That can lead to loss of precision in BO's whose size is > > > > > > not a multiple of a MiB. > > > > > > > > > > > > Fix it by changing the unit selection criteria. > > > > > > > > > > > > For much bigger BO's, their size will naturally be aligned on > > > > > > something > > > > > > bigger than a 4 KiB page, so in practice it is very unlikely their > > > > > > display > > > > > > unit would default to KiB. > > > > > > > > > > Let's wait for Rob's opinion on this. > > > > > > > > This would mean that if you have SZ_1G + SZ_1K worth of buffers, you'd > > > > report the result in KiB.. which seems like overkill to me, esp given > > > > that the result is just a snapshot in time of a figure that > > > > realistically is dynamic. > > > > Yeah, my point was that, generally, such big buffers tend to have > > a bigger size alignment (like 2MB for anything bigger than 1GB), but > > maybe this assumption doesn't stand for all drivers. > > Maybe for CMA? Regardless, this # is the sum of buffer sizes, so you > could still get that 1G+1K scenario My bad, for some reason I had per-buffer size printing in mind.
Re: [Freedreno] [PATCH v4 6/6] drm/drm-file: Show finer-grained BO sizes in drm_show_memory_stats
On Wed, Sep 13, 2023 at 12:36 AM Boris Brezillon
wrote:
>
> On Tue, 12 Sep 2023 19:14:35 -0700
> Rob Clark wrote:
>
> > On Tue, Sep 12, 2023 at 6:46 PM Rob Clark wrote:
> > >
> > > On Tue, Sep 12, 2023 at 2:32 AM Boris Brezillon
> > > wrote:
> > > >
> > > > On Tue, 12 Sep 2023 09:37:00 +0100
> > > > Adrián Larumbe wrote:
> > > >
> > > > > The current implementation will try to pick the highest available size
> > > > > display unit as soon as the BO size exceeds that of the previous
> > > > > multiplier. That can lead to loss of precision in BO's whose size is
> > > > > not a multiple of a MiB.
> > > > >
> > > > > Fix it by changing the unit selection criteria.
> > > > >
> > > > > For much bigger BO's, their size will naturally be aligned on
> > > > > something
> > > > > bigger than a 4 KiB page, so in practice it is very unlikely their
> > > > > display
> > > > > unit would default to KiB.
> > > >
> > > > Let's wait for Rob's opinion on this.
> > >
> > > This would mean that if you have SZ_1G + SZ_1K worth of buffers, you'd
> > > report the result in KiB.. which seems like overkill to me, esp given
> > > that the result is just a snapshot in time of a figure that
> > > realistically is dynamic.
>
> Yeah, my point was that, generally, such big buffers tend to have
> a bigger size alignment (like 2MB for anything bigger than 1GB), but
> maybe this assumption doesn't stand for all drivers.
Maybe for CMA? Regardless, this # is the sum of buffer sizes, so you
could still get that 1G+1K scenario
> > >
> > > Maybe if you have SZ_1G+SZ_1K worth of buffers you should report the
> > > result with more precision than GiB, but more than MiB seems a bit
> > > overkill.
> > >
> > > BR,
> > > -R
> > >
> > > > >
> > > > > Signed-off-by: Adrián Larumbe
> > > > > ---
> > > > > drivers/gpu/drm/drm_file.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > > > > index 762965e3d503..bf7d2fe46bfa 100644
> > > > > --- a/drivers/gpu/drm/drm_file.c
> > > > > +++ b/drivers/gpu/drm/drm_file.c
> > > > > @@ -879,7 +879,7 @@ static void print_size(struct drm_printer *p,
> > > > > const char *stat,
> > > > > unsigned u;
> > > > >
> > > > > for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
> > > > > - if (sz < SZ_1K)
> >
> > btw, I was thinking more along the lines of:
> >
> >if (sz < 10*SZ_1K)
> >
> > (or perhaps maybe 100*SZ_1K)
>
> I think I suggested doing that at some point:
>
> if ((sz & (SZ_1K - 1)) &&
> sz < UPPER_UNIT_THRESHOLD * SZ_1K)
> break;
>
> so we can keep using the upper unit if the size is a multiple of this
> upper unit, even if it's smaller than the selected threshold.
yeah, that wfm
BR,
-R
>
> >
> > I mean, any visualization tool is going to scale the y axis based on
> > the order of magnitude.. and if I'm looking at the fdinfo with my
> > eyeballs I don't want to count the # of digits manually to do the
> > conversion in my head. The difference btwn 4 or 5 or maybe 6 digits
> > is easy enough to eyeball, but more than that is too much for my
> > eyesight, and I'm not seeing how it is useful ;-)
> >
> > But if someone really has a valid use case for having precision in 1KB
> > then I'm willing to be overruled.
>
> So, precision loss was one aspect, but my main concern was having
> things displayed in KiB when they could have been displayed in MiB,
> because the size is a multiple of a MiB but still not big enough to
> pass the threshold test (which was set to 1x in the previous
> version).
>
> > But I'm not a fan of the earlier
> > approach of different drivers reporting results differently, the whole
> > point of fdinfo was to have some standardized reporting.
>
> Totally agree with that.
>
> >
> > BR,
> > -R
> >
> > > > > + if (sz & (SZ_1K - 1))
> > > > > break;
> > > > > sz = div_u64(sz, SZ_1K);
> > > > > }
> > > >
>
Re: [Freedreno] [RFC PATCH v1 01/12] Revert "drm/sysfs: Link DRM connectors to corresponding Type-C connectors"
On Wed, 13 Sept 2023 at 16:15, Heikki Krogerus wrote: > > On Wed, Sep 13, 2023 at 01:26:14PM +0300, Dmitry Baryshkov wrote: > > Hi Heikki, > > > > On Wed, 13 Sept 2023 at 12:27, Heikki Krogerus > > wrote: > > > On Tue, Sep 12, 2023 at 08:39:45PM +0300, Dmitry Baryshkov wrote: > > > > On 12/09/2023 14:05, Heikki Krogerus wrote: > > > > > On Tue, Sep 12, 2023 at 12:15:10AM +0300, Dmitry Baryshkov wrote: > > > > > > On 06/09/2023 16:38, Heikki Krogerus wrote: > > > > > > > On Wed, Sep 06, 2023 at 03:48:35PM +0300, Dmitry Baryshkov wrote: > > > > > > > > On Wed, 6 Sept 2023 at 15:44, Heikki Krogerus > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Tue, Sep 05, 2023 at 01:56:59PM +0300, Dmitry Baryshkov > > > > > > > > > wrote: > > > > > > > > > > Hi Heikki, > > > > > > > > > > > > > > > > > > > > On Tue, 5 Sept 2023 at 11:50, Heikki Krogerus > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Dmitry, > > > > > > > > > > > > > > > > > > > > > > On Mon, Sep 04, 2023 at 12:41:39AM +0300, Dmitry > > > > > > > > > > > Baryshkov wrote: > > > > > > > > > > > > The kdev->fwnode pointer is never set in > > > > > > > > > > > > drm_sysfs_connector_add(), so > > > > > > > > > > > > dev_fwnode() checks never succeed, making the > > > > > > > > > > > > respective commit NOP. > > > > > > > > > > > > > > > > > > > > > > That's not true. The dev->fwnode is assigned when the > > > > > > > > > > > device is > > > > > > > > > > > created on ACPI platforms automatically. If the > > > > > > > > > > > drm_connector fwnode > > > > > > > > > > > member is assigned before the device is registered, then > > > > > > > > > > > that fwnode > > > > > > > > > > > is assigned also to the device - see > > > > > > > > > > > drm_connector_acpi_find_companion(). > > > > > > > > > > > > > > > > > > > > > > But please note that even if drm_connector does not have > > > > > > > > > > > anything in > > > > > > > > > > > its fwnode member, the device may still be assigned > > > > > > > > > > > fwnode, just based > > > > > > > > > > > on some other logic (maybe in drivers/acpi/acpi_video.c?). > > > > > > > > > > > > > > > > > > > > > > > And if drm_sysfs_connector_add() is modified to set > > > > > > > > > > > > kdev->fwnode, it > > > > > > > > > > > > breaks drivers already using components (as it was > > > > > > > > > > > > pointed at [1]), > > > > > > > > > > > > resulting in a deadlock. Lockdep trace is provided > > > > > > > > > > > > below. > > > > > > > > > > > > > > > > > > > > > > > > Granted these two issues, it seems impractical to fix > > > > > > > > > > > > this commit in any > > > > > > > > > > > > sane way. Revert it instead. > > > > > > > > > > > > > > > > > > > > > > I think there is already user space stuff that relies on > > > > > > > > > > > these links, > > > > > > > > > > > so I'm not sure you can just remove them like that. If > > > > > > > > > > > the component > > > > > > > > > > > framework is not the correct tool here, then I think you > > > > > > > > > > > need to > > > > > > > > > > > suggest some other way of creating them. > > > > > > > > > > > > > > > > > > > > The issue (that was pointed out during review) is that > > > > > > > > > > having a > > > > > > > > > > component code in the framework code can lead to lockups. > > > > > > > > > > With the > > > > > > > > > > patch #2 in place (which is the only logical way to set > > > > > > > > > > kdev->fwnode > > > > > > > > > > for non-ACPI systems) probing of drivers which use > > > > > > > > > > components and set > > > > > > > > > > drm_connector::fwnode breaks immediately. > > > > > > > > > > > > > > > > > > > > Can we move the component part to the respective drivers? > > > > > > > > > > With the > > > > > > > > > > patch 2 in place, connector->fwnode will be copied to the > > > > > > > > > > created > > > > > > > > > > kdev's fwnode pointer. > > > > > > > > > > > > > > > > > > > > Another option might be to make this drm_sysfs component > > > > > > > > > > registration optional. > > > > > > > > > > > > > > > > > > You don't need to use the component framework at all if there > > > > > > > > > is > > > > > > > > > a better way of determining the connection between the DP and > > > > > > > > > its > > > > > > > > > Type-C connector (I'm assuming that that's what this series > > > > > > > > > is about). > > > > > > > > > You just need the symlinks, not the component. > > > > > > > > > > > > > > > > The problem is that right now this component registration has > > > > > > > > become > > > > > > > > mandatory. And if I set the kdev->fwnode manually (like in the > > > > > > > > patch > > > > > > > > 2), the kernel hangs inside the component code. > > > > > > > > That's why I proposed to move the components to the place where > > > > > > > > they > > > > > > > > are really necessary, e.g. i915 and amd drivers. > > > > > > > > > > > > > > So why can't we replace the component with the method you are > > > >
Re: [Freedreno] [RFC PATCH v1 01/12] Revert "drm/sysfs: Link DRM connectors to corresponding Type-C connectors"
Hi Heikki, On Wed, 13 Sept 2023 at 12:27, Heikki Krogerus wrote: > On Tue, Sep 12, 2023 at 08:39:45PM +0300, Dmitry Baryshkov wrote: > > On 12/09/2023 14:05, Heikki Krogerus wrote: > > > On Tue, Sep 12, 2023 at 12:15:10AM +0300, Dmitry Baryshkov wrote: > > > > On 06/09/2023 16:38, Heikki Krogerus wrote: > > > > > On Wed, Sep 06, 2023 at 03:48:35PM +0300, Dmitry Baryshkov wrote: > > > > > > On Wed, 6 Sept 2023 at 15:44, Heikki Krogerus > > > > > > wrote: > > > > > > > > > > > > > > On Tue, Sep 05, 2023 at 01:56:59PM +0300, Dmitry Baryshkov wrote: > > > > > > > > Hi Heikki, > > > > > > > > > > > > > > > > On Tue, 5 Sept 2023 at 11:50, Heikki Krogerus > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Dmitry, > > > > > > > > > > > > > > > > > > On Mon, Sep 04, 2023 at 12:41:39AM +0300, Dmitry Baryshkov > > > > > > > > > wrote: > > > > > > > > > > The kdev->fwnode pointer is never set in > > > > > > > > > > drm_sysfs_connector_add(), so > > > > > > > > > > dev_fwnode() checks never succeed, making the respective > > > > > > > > > > commit NOP. > > > > > > > > > > > > > > > > > > That's not true. The dev->fwnode is assigned when the device > > > > > > > > > is > > > > > > > > > created on ACPI platforms automatically. If the drm_connector > > > > > > > > > fwnode > > > > > > > > > member is assigned before the device is registered, then that > > > > > > > > > fwnode > > > > > > > > > is assigned also to the device - see > > > > > > > > > drm_connector_acpi_find_companion(). > > > > > > > > > > > > > > > > > > But please note that even if drm_connector does not have > > > > > > > > > anything in > > > > > > > > > its fwnode member, the device may still be assigned fwnode, > > > > > > > > > just based > > > > > > > > > on some other logic (maybe in drivers/acpi/acpi_video.c?). > > > > > > > > > > > > > > > > > > > And if drm_sysfs_connector_add() is modified to set > > > > > > > > > > kdev->fwnode, it > > > > > > > > > > breaks drivers already using components (as it was pointed > > > > > > > > > > at [1]), > > > > > > > > > > resulting in a deadlock. Lockdep trace is provided below. > > > > > > > > > > > > > > > > > > > > Granted these two issues, it seems impractical to fix this > > > > > > > > > > commit in any > > > > > > > > > > sane way. Revert it instead. > > > > > > > > > > > > > > > > > > I think there is already user space stuff that relies on > > > > > > > > > these links, > > > > > > > > > so I'm not sure you can just remove them like that. If the > > > > > > > > > component > > > > > > > > > framework is not the correct tool here, then I think you need > > > > > > > > > to > > > > > > > > > suggest some other way of creating them. > > > > > > > > > > > > > > > > The issue (that was pointed out during review) is that having a > > > > > > > > component code in the framework code can lead to lockups. With > > > > > > > > the > > > > > > > > patch #2 in place (which is the only logical way to set > > > > > > > > kdev->fwnode > > > > > > > > for non-ACPI systems) probing of drivers which use components > > > > > > > > and set > > > > > > > > drm_connector::fwnode breaks immediately. > > > > > > > > > > > > > > > > Can we move the component part to the respective drivers? With > > > > > > > > the > > > > > > > > patch 2 in place, connector->fwnode will be copied to the > > > > > > > > created > > > > > > > > kdev's fwnode pointer. > > > > > > > > > > > > > > > > Another option might be to make this drm_sysfs component > > > > > > > > registration optional. > > > > > > > > > > > > > > You don't need to use the component framework at all if there is > > > > > > > a better way of determining the connection between the DP and its > > > > > > > Type-C connector (I'm assuming that that's what this series is > > > > > > > about). > > > > > > > You just need the symlinks, not the component. > > > > > > > > > > > > The problem is that right now this component registration has become > > > > > > mandatory. And if I set the kdev->fwnode manually (like in the patch > > > > > > 2), the kernel hangs inside the component code. > > > > > > That's why I proposed to move the components to the place where they > > > > > > are really necessary, e.g. i915 and amd drivers. > > > > > > > > > > So why can't we replace the component with the method you are > > > > > proposing in this series of finding out the Type-C port also with > > > > > i915, AMD, or whatever driver and platform (that's the only thing that > > > > > component is used for)? > > > > > > > > The drm/msm driver uses drm_bridge for the pipeline (including the last > > > > DP > > > > entry) and the drm_bridge_connector to create the connector. I think > > > > that > > > > enabling i915 and AMD drivers to use drm_bridge fells out of scope for > > > > this > > > > series. > > > > > > > > > > > > > Determining the connection between a DP and its Type-C connector is > > > > > starting to get really important, so ideally we have a
Re: [Freedreno] [RFC PATCH v1 01/12] Revert "drm/sysfs: Link DRM connectors to corresponding Type-C connectors"
On 12/09/2023 19:39, Dmitry Baryshkov wrote: On 12/09/2023 14:05, Heikki Krogerus wrote: On Tue, Sep 12, 2023 at 12:15:10AM +0300, Dmitry Baryshkov wrote: On 06/09/2023 16:38, Heikki Krogerus wrote: On Wed, Sep 06, 2023 at 03:48:35PM +0300, Dmitry Baryshkov wrote: On Wed, 6 Sept 2023 at 15:44, Heikki Krogerus wrote: On Tue, Sep 05, 2023 at 01:56:59PM +0300, Dmitry Baryshkov wrote: Hi Heikki, On Tue, 5 Sept 2023 at 11:50, Heikki Krogerus wrote: Hi Dmitry, On Mon, Sep 04, 2023 at 12:41:39AM +0300, Dmitry Baryshkov wrote: The kdev->fwnode pointer is never set in drm_sysfs_connector_add(), so dev_fwnode() checks never succeed, making the respective commit NOP. That's not true. The dev->fwnode is assigned when the device is created on ACPI platforms automatically. If the drm_connector fwnode member is assigned before the device is registered, then that fwnode is assigned also to the device - see drm_connector_acpi_find_companion(). But please note that even if drm_connector does not have anything in its fwnode member, the device may still be assigned fwnode, just based on some other logic (maybe in drivers/acpi/acpi_video.c?). And if drm_sysfs_connector_add() is modified to set kdev->fwnode, it breaks drivers already using components (as it was pointed at [1]), resulting in a deadlock. Lockdep trace is provided below. Granted these two issues, it seems impractical to fix this commit in any sane way. Revert it instead. I think there is already user space stuff that relies on these links, so I'm not sure you can just remove them like that. If the component framework is not the correct tool here, then I think you need to suggest some other way of creating them. The issue (that was pointed out during review) is that having a component code in the framework code can lead to lockups. With the patch #2 in place (which is the only logical way to set kdev->fwnode for non-ACPI systems) probing of drivers which use components and set drm_connector::fwnode breaks immediately. Can we move the component part to the respective drivers? With the patch 2 in place, connector->fwnode will be copied to the created kdev's fwnode pointer. Another option might be to make this drm_sysfs component registration optional. You don't need to use the component framework at all if there is a better way of determining the connection between the DP and its Type-C connector (I'm assuming that that's what this series is about). You just need the symlinks, not the component. The problem is that right now this component registration has become mandatory. And if I set the kdev->fwnode manually (like in the patch 2), the kernel hangs inside the component code. That's why I proposed to move the components to the place where they are really necessary, e.g. i915 and amd drivers. So why can't we replace the component with the method you are proposing in this series of finding out the Type-C port also with i915, AMD, or whatever driver and platform (that's the only thing that component is used for)? The drm/msm driver uses drm_bridge for the pipeline (including the last DP entry) and the drm_bridge_connector to create the connector. I think that enabling i915 and AMD drivers to use drm_bridge fells out of scope for this series. Determining the connection between a DP and its Type-C connector is starting to get really important, so ideally we have a common solution for that. Yes. This is what we have been discussing with Simon for quite some time on #dri-devel. Unfortunately I think the solution that got merged was pretty much hastened in instead of being well-thought. For example, it is also not always possible to provide the drm_connector / typec_connector links (as you can see from the patch7. Sometimes we can only express that this is a Type-C DP connector, but we can not easily point it to the particular USB-C port. So, I'm not sure, how can we proceed here. Currently merged patch breaks drm/msm if we even try to use it by setting kdef->fwnode to drm_connector->fwnode. The pointed out `drivers/usb/typec/port-mapper.c` is an ACPI-only thing, which is not expected to work in a non-ACPI cases. You really have to always supply not only the Type-C ports and partners, but also the alt modes. You need them, firstly to keep things sane inside kernel, but more importantly, so they are always exposed to the user space, AND, always the same way. We have ABIs for all this stuff, including the DP alt mode. Use them. No shortcuts. So here's what you need to do. UCSI does not seem to bring you anything useful, so just disable it for now. You don't need it. Your port driver is clearly drivers/soc/qcom/pmic_glink_altmode.c, so that's where you need to register all these components - the ports, partners and alt modes. You have all the needed information there. To make things even more complicate, UCSI is necessary for the USB part of the story. It handles vbus and direction. On new platforms (starting from SM8450)
Re: [Freedreno] [PATCH v4 6/6] drm/drm-file: Show finer-grained BO sizes in drm_show_memory_stats
On Tue, 12 Sep 2023 19:14:35 -0700
Rob Clark wrote:
> On Tue, Sep 12, 2023 at 6:46 PM Rob Clark wrote:
> >
> > On Tue, Sep 12, 2023 at 2:32 AM Boris Brezillon
> > wrote:
> > >
> > > On Tue, 12 Sep 2023 09:37:00 +0100
> > > Adrián Larumbe wrote:
> > >
> > > > The current implementation will try to pick the highest available size
> > > > display unit as soon as the BO size exceeds that of the previous
> > > > multiplier. That can lead to loss of precision in BO's whose size is
> > > > not a multiple of a MiB.
> > > >
> > > > Fix it by changing the unit selection criteria.
> > > >
> > > > For much bigger BO's, their size will naturally be aligned on something
> > > > bigger than a 4 KiB page, so in practice it is very unlikely their
> > > > display
> > > > unit would default to KiB.
> > >
> > > Let's wait for Rob's opinion on this.
> >
> > This would mean that if you have SZ_1G + SZ_1K worth of buffers, you'd
> > report the result in KiB.. which seems like overkill to me, esp given
> > that the result is just a snapshot in time of a figure that
> > realistically is dynamic.
Yeah, my point was that, generally, such big buffers tend to have
a bigger size alignment (like 2MB for anything bigger than 1GB), but
maybe this assumption doesn't stand for all drivers.
> >
> > Maybe if you have SZ_1G+SZ_1K worth of buffers you should report the
> > result with more precision than GiB, but more than MiB seems a bit
> > overkill.
> >
> > BR,
> > -R
> >
> > > >
> > > > Signed-off-by: Adrián Larumbe
> > > > ---
> > > > drivers/gpu/drm/drm_file.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> > > > index 762965e3d503..bf7d2fe46bfa 100644
> > > > --- a/drivers/gpu/drm/drm_file.c
> > > > +++ b/drivers/gpu/drm/drm_file.c
> > > > @@ -879,7 +879,7 @@ static void print_size(struct drm_printer *p, const
> > > > char *stat,
> > > > unsigned u;
> > > >
> > > > for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
> > > > - if (sz < SZ_1K)
>
> btw, I was thinking more along the lines of:
>
>if (sz < 10*SZ_1K)
>
> (or perhaps maybe 100*SZ_1K)
I think I suggested doing that at some point:
if ((sz & (SZ_1K - 1)) &&
sz < UPPER_UNIT_THRESHOLD * SZ_1K)
break;
so we can keep using the upper unit if the size is a multiple of this
upper unit, even if it's smaller than the selected threshold.
>
> I mean, any visualization tool is going to scale the y axis based on
> the order of magnitude.. and if I'm looking at the fdinfo with my
> eyeballs I don't want to count the # of digits manually to do the
> conversion in my head. The difference btwn 4 or 5 or maybe 6 digits
> is easy enough to eyeball, but more than that is too much for my
> eyesight, and I'm not seeing how it is useful ;-)
>
> But if someone really has a valid use case for having precision in 1KB
> then I'm willing to be overruled.
So, precision loss was one aspect, but my main concern was having
things displayed in KiB when they could have been displayed in MiB,
because the size is a multiple of a MiB but still not big enough to
pass the threshold test (which was set to 1x in the previous
version).
> But I'm not a fan of the earlier
> approach of different drivers reporting results differently, the whole
> point of fdinfo was to have some standardized reporting.
Totally agree with that.
>
> BR,
> -R
>
> > > > + if (sz & (SZ_1K - 1))
> > > > break;
> > > > sz = div_u64(sz, SZ_1K);
> > > > }
> > >
