date:20230630

Re: [Freedreno] [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property

2023-06-30 Thread Jessica Zhang





On 6/29/2023 5:59 PM, Dmitry Baryshkov wrote:

On 30/06/2023 03:25, Jessica Zhang wrote:

Drop DPU_PLANE_COLOR_FILL_FLAG and check the DRM solid_fill property to
determine if the plane is solid fill. In addition drop the DPU plane
color_fill field as we can now use drm_plane_state.solid_fill instead,
and pass in drm_plane_state.alpha to _dpu_plane_color_fill_pipe() to
allow userspace to configure the alpha value for the solid fill color.

Signed-off-by: Jessica Zhang 


Reviewed-by: Dmitry Baryshkov 

Minor suggestion below.


---
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 21 +++--
  1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c

index 4476722f03bb..11d4fb771a1f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -42,7 +42,6 @@
  #define SHARP_SMOOTH_THR_DEFAULT    8
  #define SHARP_NOISE_THR_DEFAULT    2
-#define DPU_PLANE_COLOR_FILL_FLAG    BIT(31)
  #define DPU_ZPOS_MAX 255
  /*
@@ -82,7 +81,6 @@ struct dpu_plane {
  enum dpu_sspp pipe;
-    uint32_t color_fill;
  bool is_error;
  bool is_rt_pipe;
  const struct dpu_mdss_cfg *catalog;
@@ -606,6 +604,17 @@ static void _dpu_plane_color_fill_pipe(struct 
dpu_plane_state *pstate,
  _dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, 
pstate->rotation);

  }
+static uint32_t _dpu_plane_get_fill_color(struct drm_solid_fill 
solid_fill)


Please consider accepting drm_plane_state instead and handling alpha 
here. Then _dpu_color_fill can accept rgba colour instead of separate 
RGB and alpha values.


Hi Dmitry,

Do you mean adding a patch to refactor _dpu_plane_color_fill() to accept 
an RGBA color?


Since, currently, the `color` parameter gets truncated to a BGR888 value 
and OR'd with the `alpha` parameter [1].


Thanks,

Jessica Zhang

[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L682





+{
+    uint32_t ret = 0;
+
+    ret |= ((uint8_t) solid_fill.b) << 16;
+    ret |= ((uint8_t) solid_fill.g) << 8;
+    ret |= ((uint8_t) solid_fill.r);
+
+    return ret;
+}
+
  /**
   * _dpu_plane_color_fill - enables color fill on plane
   * @pdpu:   Pointer to DPU plane object
@@ -977,9 +986,9 @@ void dpu_plane_flush(struct drm_plane *plane)
  if (pdpu->is_error)
  /* force white frame with 100% alpha pipe output on error */
  _dpu_plane_color_fill(pdpu, 0xFF, 0xFF);
-    else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
-    /* force 100% alpha */
-    _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
+    else if (drm_plane_solid_fill_enabled(plane->state))
+    _dpu_plane_color_fill(pdpu, 
_dpu_plane_get_fill_color(plane->state->solid_fill),

+    plane->state->alpha);
  else {
  dpu_plane_flush_csc(pdpu, &pstate->pipe);
  dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
@@ -1024,7 +1033,7 @@ static void dpu_plane_sspp_update_pipe(struct 
drm_plane *plane,

  }
  /* override for color fill */
-    if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
+    if (drm_plane_solid_fill_enabled(plane->state)) {
  _dpu_plane_set_qos_ctrl(plane, pipe, false);
  /* skip remaining processing on color fill */



--
With best wishes
Dmitry

Re: [Freedreno] [PATCH RFC v4 6/7] drm/msm/dpu: Allow NULL FBs in atomic commit

2023-06-30 Thread Jessica Zhang





On 6/30/2023 1:21 AM, Pekka Paalanen wrote:

On Fri, 30 Jun 2023 03:52:37 +0300
Dmitry Baryshkov  wrote:


On 30/06/2023 03:25, Jessica Zhang wrote:

Since solid fill planes allow for a NULL framebuffer in a valid commit,
add NULL framebuffer checks to atomic commit calls within DPU.

Signed-off-by: Jessica Zhang 
---
   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  9 ++-
   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 45 
+++
   2 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 1edf2b6b0a26..d1b37d2cc202 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -451,6 +451,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
struct drm_plane_state *state;
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
struct dpu_plane_state *pstate = NULL;
+   const struct msm_format *fmt;
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
   
@@ -470,7 +471,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,

pstate = to_dpu_plane_state(state);
fb = state->fb;
   
-		format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));

+   if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb)
+   fmt = msm_framebuffer_format(pstate->base.fb);
+   else
+   fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
+   DRM_FORMAT_RGBA, 0);


The DRM_FORMAT_RGBA should be defined somewhere in patch 1 as format
for the solid_fill, then that define can be used in this patch.


Isn't this just a driver-specific decision to convert a RGB323232
solid_fill to be presented as a RGBA?


Hi Dmitry and Pekka,

Yes, the ABGR format is specific to msm/dpu. In earlier revisions of 
the series, we had come to an agreement that the solid fill property 
should take RGB323232 to match the similar Wayland single pixel buffer 
protocol [1].


[1] 
https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/104




Though, below there is ABGR used for something... inconsistent?


Typo on my part. The format should be consistently ABGR.

Thanks,

Jessica Zhang




Thanks,
pq




+
+   format = to_dpu_format(fmt);
   
   		if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)

bg_alpha_enable = true;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 5f0984ce62b1..4476722f03bb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -837,8 +837,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
   
   	pipe_cfg->dst_rect = new_plane_state->dst;
   
-	fb_rect.x2 = new_plane_state->fb->width;

-   fb_rect.y2 = new_plane_state->fb->height;
+   if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && 
new_plane_state->fb) {
+   fb_rect.x2 = new_plane_state->fb->width;
+   fb_rect.y2 = new_plane_state->fb->height;
+   }
   
   	/* Ensure fb size is supported */

if (drm_rect_width(&fb_rect) > MAX_IMG_WIDTH ||
@@ -848,10 +850,13 @@ 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;
   
+	if (drm_plane_solid_fill_enabled(new_plane_state))

+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   else
+   fmt = 
to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+
if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
/*
 * In parallel multirect case only the half of the usual width
@@ -1082,21 +1087,32 @@ static void dpu_plane_sspp_atomic_update(struct 
drm_plane *plane)
struct drm_crtc *crtc = state->crtc;
struct drm_framebuffer *fb = state->fb;
bool is_rt_pipe;
-   const struct dpu_format *fmt =
-   to_dpu_format(msm_framebuffer_format(fb));
+   const struct dpu_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
struct msm_gem_address_space *aspace = kms->base.aspace;
struct dpu_hw_fmt_layout layout;
bool layout_valid = false;
-   int ret;
   
-	ret = dpu_format_populate_layout(aspace, fb, &layout);

-   if (ret)
-   DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
-   else
-   layout_valid = true;
+   if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB

Re: [Freedreno] [PATCH v2] drm/msm/adreno: Assign revn to A635

2023-06-30 Thread Dmitry Baryshkov

On Sat, 1 Jul 2023 at 02:12, Konrad Dybcio  wrote:
>
> Recently, a WARN_ON() was introduced to ensure that revn is filled before
> adreno_is_aXYZ is called. This however doesn't work very well when revn is
> 0 by design (such as for A635). Fill it in as a stopgap solution for
> -fixes.
>
> Fixes: cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified before 
> being set")
> Signed-off-by: Konrad Dybcio 

As the v1:

Reviewed-by: Dmitry Baryshkov 

> ---
> Changes in v2:
> - add fixes
> - Link to v1: 
> https://lore.kernel.org/r/20230628-topic-a635-v1-1-5056e09c0...@linaro.org
> ---
>  drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index cb94cfd137a8..8ea7eae9fc52 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -345,6 +345,7 @@ static const struct adreno_info gpulist[] = {
> .address_space_size = SZ_16G,
> }, {
> .rev = ADRENO_REV(6, 3, 5, ANY_ID),
> +   .revn = 635,
> .fw = {
> [ADRENO_FW_SQE] = "a660_sqe.fw",
> [ADRENO_FW_GMU] = "a660_gmu.bin",
>
> ---
> base-commit: 5c875096d59010cee4e00da1f9c7bdb07a025dc2
> change-id: 20230628-topic-a635-1b3c2c987417
>
> Best regards,
> --
> Konrad Dybcio 
>


-- 
With best wishes
Dmitry

Re: [Freedreno] [PATCH RFC v4 5/7] drm/msm/dpu: Add solid fill and pixel source properties

2023-06-30 Thread Jessica Zhang





On 6/29/2023 5:49 PM, Dmitry Baryshkov wrote:

On 30/06/2023 03:25, Jessica Zhang wrote:

Add solid_fill and pixel_source properties to DPU plane

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 ++
  1 file changed, 2 insertions(+)


This should be the last commit.


Hi Dmitry,

Acked, will move this to the end.

Thanks,

Jessica Zhang


Otherwise:

Reviewed-by: Dmitry Baryshkov 



diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c

index c2aaaded07ed..5f0984ce62b1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1429,6 +1429,8 @@ struct drm_plane *dpu_plane_init(struct 
drm_device *dev,

  DPU_ERROR("failed to install zpos property, rc = %d\n", ret);
  drm_plane_create_alpha_property(plane);
+    drm_plane_create_solid_fill_property(plane);
+    drm_plane_create_pixel_source_property(plane, 
BIT(DRM_PLANE_PIXEL_SOURCE_COLOR));

  drm_plane_create_blend_mode_property(plane,
  BIT(DRM_MODE_BLEND_PIXEL_NONE) |
  BIT(DRM_MODE_BLEND_PREMULTI) |



--
With best wishes
Dmitry

Re: [Freedreno] [PATCH RFC v4 4/7] drm/atomic: Loosen FB atomic checks

2023-06-30 Thread Jessica Zhang





On 6/29/2023 5:48 PM, Dmitry Baryshkov wrote:

On 30/06/2023 03:25, Jessica Zhang wrote:

Loosen the requirements for atomic and legacy commit so that, in cases
where solid fill planes is enabled but no FB is set, the commit can
still go through.

This includes adding framebuffer NULL checks in other areas to account
for FB being NULL when solid fill is enabled.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic.c    | 14 +++---
  drivers/gpu/drm/drm_atomic_helper.c | 34 
--

  drivers/gpu/drm/drm_plane.c |  8 
  include/drm/drm_atomic_helper.h |  4 ++--
  include/drm/drm_plane.h | 28 
  5 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 404b984d2d9f..ec9681c25366 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -668,14 +668,14 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  const struct drm_framebuffer *fb = new_plane_state->fb;
  int ret;
-    /* either *both* CRTC and FB must be set, or neither */
-    if (crtc && !fb) {
-    drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no FB\n",
+    /* either *both* CRTC and pixel source must be set, or neither */
+    if (crtc && !drm_plane_has_visible_data(new_plane_state)) {
+    drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no 
visible data\n",

 plane->base.id, plane->name);
  return -EINVAL;
-    } else if (fb && !crtc) {
-    drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] FB set but no CRTC\n",
-   plane->base.id, plane->name);
+    } else if (drm_plane_has_visible_data(new_plane_state) && !crtc) {
+    drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] Source %d has 
visible data but no CRTC\n",
+   plane->base.id, plane->name, 
new_plane_state->pixel_source);

  return -EINVAL;
  }
@@ -706,7 +706,7 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  }
-    if (fb) {
+    if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && 
fb) {

  ret = drm_atomic_check_fb(new_plane_state);
  if (ret)
  return ret;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c

index 41b8066f61ff..d05ec9ef2b3e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -864,7 +864,7 @@ int drm_atomic_helper_check_plane_state(struct 
drm_plane_state *plane_state,

  *src = drm_plane_state_src(plane_state);
  *dst = drm_plane_state_dest(plane_state);
-    if (!fb) {
+    if (!drm_plane_has_visible_data(plane_state)) {
  plane_state->visible = false;
  return 0;
  }
@@ -881,25 +881,31 @@ int drm_atomic_helper_check_plane_state(struct 
drm_plane_state *plane_state,

  return -EINVAL;
  }
-    drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+    if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) {
+    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: ", &plane_state->src, true);
-    drm_rect_debug_print("dst: ", &plane_state->dst, false);
-    return -ERANGE;
+    /* 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: ", &plane_state->src, true);
+    drm_rect_debug_print("dst: ", &plane_state->dst, false);
+    return -ERANGE;
+    }
  }
  if (crtc_state->enable)
  drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
-    plane_state->visible = drm_rect_clip_scaled(src, dst, &clip);
-
-    drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, 
rotation);

+    if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) {
+    plane_state->visible = drm_rect_clip_scaled(src, dst, &clip);
+    drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, 
rotation);

+    } else if (drm_plane_solid_fill_enabled(plane_state)) {
+    plane_state->visible = true;
+    }
  if (!plane_state->visible)
  /*
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 24e7998d1731..5f19a27ba182 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@

Re: [Freedreno] [PATCH 0/6] Add support to print sub block registers in dpu hw catalog

2023-06-30 Thread Jessica Zhang





On 6/23/2023 12:10 AM, Marijn Suijten wrote:

It is nice if cover letters also include the subsystem:

drm/msm: Add support to print DPU sub-block registers

On 2023-06-22 16:48:52, Ryan McCann wrote:

The purpose of this patch series is to add support to print the registers
of sub blocks in the dpu hardware catalog and fix the order in which all


Hey Marijn,

Sorry for the late response -- had to shift focus onto another feature 
for a bit.




Global nit: I think we previously settled on "sblk" or "sub-block(s)",
not "sub blocks".

And capitalize DPU.


Acked.




hardware blocks are dumped for a device core dump. This involves:

1. Changing data structure from stack to queue to fix the printing order
of the device core dump.

2. Removing redundant suffix of sub block names.

3. Removing redundant prefix of sub block names.

4. Eliminating unused variable from relevant macros.


Dmitry has been doing that in one of his DPU catalog reworks.


Got it. Is there a specific one that's making similar changes? IIRC, I 
didn't see one that was changing the *_SBLK macro.





5. Defining names for sub blocks that have not yet been defined.


Let's see what this means, because the code logic should already be able
to figure this out (and in some places we can perhaps delete the name
entirely).


6. Implementing wrapper function that prints the registers of sub blocks
when there is a need.


Thought this could be rather automated, but let me see what it means in
the patches.


Sample Output of the sspp_0 block and its sub blocks for devcore dump:
==sspp_0==
...registers
...
sspp_0_scaler


My suggestion would be to put less emphasis on this header with:

 sspp_0_scaler

So that it becomes obvious that this is a sblk of the sspp_0
above.


FWIW, I think having the main block name prefix in the sblk header makes 
it clear which blocks are main block and which ones are sblks.


In addition, I'd like to keep this change within DPU (aside from the fix 
in the first patch) since implementing this change would require 
changing the *_add_block() parameters, and DSI/DP don't seem to have a 
need for conditional header formatting.


Thanks,

Jessica Zhang




...
...
sspp_0_csc
...
...
next_block
...

Signed-off-by: Ryan McCann 


No need for sign-off in cover letters.

- Marijn


---
Ryan McCann (6):
   drm/msm: Update dev core dump to not print backwards
   drm/msm/dpu: Drop unused num argument from relevant macros
   drm/msm/dpu: Define names for unnamed sblks
   drm/msm/dpu: Remove redundant suffix in name of sub blocks
   drm/msm/disp: Remove redundant prefix in name of sub blocks
   drm/msm/dpu: Update dev core dump to dump registers of sub blocks

  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c|  90 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c   | 194 +++---
  drivers/gpu/drm/msm/disp/msm_disp_snapshot_util.c |   2 +-
  3 files changed, 214 insertions(+), 72 deletions(-)
---
base-commit: 710025fdedb3767655823c3a12d27d404d209f75
change-id: 20230622-devcoredump_patch-df7e8f6fd632

Best regards,
--
Ryan McCann

[Freedreno] [PATCH v2] drm/msm/adreno: Assign revn to A635

2023-06-30 Thread Konrad Dybcio

Recently, a WARN_ON() was introduced to ensure that revn is filled before
adreno_is_aXYZ is called. This however doesn't work very well when revn is
0 by design (such as for A635). Fill it in as a stopgap solution for
-fixes.

Fixes: cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified before 
being set")
Signed-off-by: Konrad Dybcio 
---
Changes in v2:
- add fixes
- Link to v1: 
https://lore.kernel.org/r/20230628-topic-a635-v1-1-5056e09c0...@linaro.org
---
 drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index cb94cfd137a8..8ea7eae9fc52 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -345,6 +345,7 @@ static const struct adreno_info gpulist[] = {
.address_space_size = SZ_16G,
}, {
.rev = ADRENO_REV(6, 3, 5, ANY_ID),
+   .revn = 635,
.fw = {
[ADRENO_FW_SQE] = "a660_sqe.fw",
[ADRENO_FW_GMU] = "a660_gmu.bin",

---
base-commit: 5c875096d59010cee4e00da1f9c7bdb07a025dc2
change-id: 20230628-topic-a635-1b3c2c987417

Best regards,
-- 
Konrad Dybcio

Re: [Freedreno] [PATCH RFC v4 2/7] drm: Introduce pixel_source DRM plane property

2023-06-30 Thread Jessica Zhang





On 6/30/2023 7:43 AM, Sebastian Wick wrote:

On Fri, Jun 30, 2023 at 2:26 AM Jessica Zhang  wrote:


Add support for pixel_source property to drm_plane and related
documentation.

This enum property will allow user to specify a pixel source for the
plane. Possible pixel sources will be defined in the
drm_plane_pixel_source enum.

The current possible pixel sources are DRM_PLANE_PIXEL_SOURCE_FB and
DRM_PLANE_PIXEL_SOURCE_COLOR. The default value is *_SOURCE_FB.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
  drivers/gpu/drm/drm_blend.c   | 81 +++
  include/drm/drm_blend.h   |  2 +
  include/drm/drm_plane.h   | 21 
  5 files changed, 109 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index fe14be2bd2b2..86fb876efbe6 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct 
drm_plane_state *plane_state,

 plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
 plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
+   plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;

 if (plane_state->solid_fill_blob) {
 drm_property_blob_put(plane_state->solid_fill_blob);
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index a28b4ee79444..6e59c21af66b 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -596,6 +596,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
*plane,
 drm_property_blob_put(solid_fill);

 return ret;
+   } else if (property == plane->pixel_source_property) {
+   state->pixel_source = val;
 } else if (property == plane->alpha_property) {
 state->alpha = val;
 } else if (property == plane->blend_mode_property) {
@@ -671,6 +673,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
 } else if (property == plane->solid_fill_property) {
 *val = state->solid_fill_blob ?
 state->solid_fill_blob->base.id : 0;
+   } else if (property == plane->pixel_source_property) {
+   *val = state->pixel_source;
 } else if (property == plane->alpha_property) {
 *val = state->alpha;
 } else if (property == plane->blend_mode_property) {
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 38c3c5d6453a..8c100a957ee2 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -189,6 +189,18 @@
   * solid_fill is set up with drm_plane_create_solid_fill_property(). It
   * contains pixel data that drivers can use to fill a plane.
   *
+ * pixel_source:
+ * pixel_source is set up with drm_plane_create_pixel_source_property().
+ * It is used to toggle the source of pixel data for the plane.
+ *
+ * Possible values:
+ *
+ * "FB":
+ * Framebuffer source
+ *
+ * "COLOR":
+ * solid_fill source
+ *
   * Note that all the property extensions described here apply either to the
   * plane or the CRTC (e.g. for the background color, which currently is not
   * exposed and assumed to be black).
@@ -648,3 +660,72 @@ int drm_plane_create_solid_fill_property(struct drm_plane 
*plane)
 return 0;
  }
  EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
+
+/**
+ * drm_plane_create_pixel_source_property - create a new pixel source property
+ * @plane: drm plane
+ * @supported_sources: bitmask of supported pixel_sources for the driver (NOT
+ * including DRM_PLANE_PIXEL_SOURCE_FB, as it will be 
supported
+ * by default).
+ *
+ * This creates a new property describing the current source of pixel data for 
the
+ * plane.
+ *
+ * The property is exposed to userspace as an enumeration property called
+ * "pixel_source" and has the following enumeration values:
+ *
+ * "FB":
+ * Framebuffer pixel source
+ *
+ * "COLOR":
+ * Solid fill color pixel source


Can we add a "NONE" value?

I know it has been discussed earlier if we *need*  this and I don't
think we do. I just think it would be better API design to disable
planes this way than having to know that a framebuffer pixel source
with a NULL framebuffer disables the plane. Obviously also keep the
old behavior for backwards compatibility.


Hi Sebastian,

Sounds good.

So if pixel_source == NONE disables the planes, would that mean cases 
where pixel_source == COLOR && solid_fill_blob == NULL, the atomic 
commit should throw an error?


Thanks,

Jessica Zhang




+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_plane_create_pixel_source_property(struct drm_plane *plane,

[Freedreno] [PATCH 09/15] drm/msm/hdmi: add runtime PM calls to DDC transfer function

2023-06-30 Thread Dmitry Baryshkov

We must be sure that the HDMI controller is powered on, while performing
the DDC transfer. Add corresponding runtime PM calls to
msm_hdmi_i2c_xfer().

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi_i2c.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
index de182c004843..9c78c6c528be 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
@@ -107,11 +107,15 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
if (num == 0)
return num;
 
+   ret = pm_runtime_resume_and_get(&hdmi->pdev->dev);
+   if (ret)
+   return ret;
+
init_ddc(hdmi_i2c);
 
ret = ddc_clear_irq(hdmi_i2c);
if (ret)
-   return ret;
+   goto fail;
 
for (i = 0; i < num; i++) {
struct i2c_msg *p = &msgs[i];
@@ -169,7 +173,7 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
hdmi_read(hdmi, REG_HDMI_DDC_SW_STATUS),
hdmi_read(hdmi, REG_HDMI_DDC_HW_STATUS),
hdmi_read(hdmi, REG_HDMI_DDC_INT_CTRL));
-   return ret;
+   goto fail;
}
 
ddc_status = hdmi_read(hdmi, REG_HDMI_DDC_SW_STATUS);
@@ -202,7 +206,13 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
}
}
 
+   pm_runtime_put(&hdmi->pdev->dev);
+
return i;
+
+fail:
+   pm_runtime_put(&hdmi->pdev->dev);
+   return ret;
 }
 
 static u32 msm_hdmi_i2c_func(struct i2c_adapter *adapter)
-- 
2.39.2

[Freedreno] [PATCH 14/15] drm/msm/hdmi: ensure that HDMI is one if HPD is requested

2023-06-30 Thread Dmitry Baryshkov

The HDMI block needs to be enabled to properly generate HPD events. Make
sure it is not turned off in the disable paths if HPD delivery is enabled.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c| 1 +
 drivers/gpu/drm/msm/hdmi/hdmi.h| 2 ++
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 8 +++-
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 9 -
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 598c8284f125..ec109255ee17 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -419,6 +419,7 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
hdmi->pdev = pdev;
hdmi->config = config;
spin_lock_init(&hdmi->reg_lock);
+   mutex_init(&hdmi->state_mutex);
 
ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, 
&hdmi->next_bridge);
if (ret && ret != -ENODEV)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 252d617939d4..e07450fbb521 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -42,6 +42,8 @@ struct hdmi {
 
/* video state: */
bool power_on;
+   bool hpd_enabled;
+   struct mutex state_mutex; /* protects two booleans */
unsigned long int pixclock;
 
void __iomem *mmio;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index d6d57768f3dd..01630445a664 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -125,11 +125,13 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct 
drm_bridge *bridge,
 
DBG("power up");
 
+   mutex_lock(&hdmi->state_mutex);
if (!hdmi->power_on) {
msm_hdmi_phy_resource_enable(phy);
msm_hdmi_power_on(bridge);
hdmi->power_on = true;
}
+   mutex_unlock(&hdmi->state_mutex);
 
if (hdmi->hdmi_mode) {
msm_hdmi_config_avi_infoframe(hdmi);
@@ -155,7 +157,10 @@ static void msm_hdmi_bridge_atomic_post_disable(struct 
drm_bridge *bridge,
msm_hdmi_hdcp_off(hdmi->hdcp_ctrl);
 
DBG("power down");
-   msm_hdmi_set_mode(hdmi, false);
+
+   /* Keep the HDMI enabled if the HPD is enabled */
+   mutex_lock(&hdmi->state_mutex);
+   msm_hdmi_set_mode(hdmi, hdmi->hpd_enabled);
 
msm_hdmi_phy_powerdown(phy);
 
@@ -166,6 +171,7 @@ static void msm_hdmi_bridge_atomic_post_disable(struct 
drm_bridge *bridge,
msm_hdmi_audio_update(hdmi);
msm_hdmi_phy_resource_disable(phy);
}
+   mutex_unlock(&hdmi->state_mutex);
 }
 
 static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge,
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index f3d6cc184999..2080e7c6700c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -73,10 +73,14 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
if (ret)
return ret;
 
+   mutex_lock(&hdmi->state_mutex);
msm_hdmi_set_mode(hdmi, false);
msm_hdmi_phy_reset(hdmi);
msm_hdmi_set_mode(hdmi, true);
 
+   hdmi->hpd_enabled = true;
+   mutex_unlock(&hdmi->state_mutex);
+
hdmi_write(hdmi, REG_HDMI_USEC_REFTIMER, 0x0001001b);
 
/* enable HPD events: */
@@ -107,7 +111,10 @@ void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge)
/* Disable HPD interrupt */
hdmi_write(hdmi, REG_HDMI_HPD_INT_CTRL, 0);
 
-   msm_hdmi_set_mode(hdmi, false);
+   mutex_lock(&hdmi->state_mutex);
+   hdmi->hpd_enabled = false;
+   msm_hdmi_set_mode(hdmi, hdmi->power_on);
+   mutex_unlock(&hdmi->state_mutex);
 
pm_runtime_put(dev);
 }
-- 
2.39.2

[Freedreno] [PATCH 15/15] drm/msm/hdmi: wire in hpd_enable/hpd_disable bridge ops

2023-06-30 Thread Dmitry Baryshkov

The HDMI driver already has msm_hdmi_hpd_enable() and
msm_hdmi_hpd_disable() functions. Wire them into the
msm_hdmi_bridge_funcs, so that HPD  can be enabled and disabled
dynamically rather than always having HPD events generation enabled.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c|  6 --
 drivers/gpu/drm/msm/hdmi/hdmi.h|  4 ++--
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  6 +++---
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 11 +--
 4 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index ec109255ee17..e1a16e20890c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -208,12 +208,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
goto fail;
}
 
-   ret = msm_hdmi_hpd_enable(hdmi->bridge);
-   if (ret < 0) {
-   DRM_DEV_ERROR(&hdmi->pdev->dev, "failed to enable HPD: %d\n", 
ret);
-   goto fail;
-   }
-
priv->bridges[priv->num_bridges++]   = hdmi->bridge;
 
return 0;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index e07450fbb521..165b85a0b0f3 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -220,8 +220,8 @@ void msm_hdmi_bridge_destroy(struct drm_bridge *bridge);
 void msm_hdmi_hpd_irq(struct drm_bridge *bridge);
 enum drm_connector_status msm_hdmi_bridge_detect(
struct drm_bridge *bridge);
-int msm_hdmi_hpd_enable(struct drm_bridge *bridge);
-void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge);
+void msm_hdmi_hpd_enable(struct drm_bridge *bridge);
+void msm_hdmi_hpd_disable(struct drm_bridge *bridge);
 
 /*
  * i2c adapter for ddc:
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 01630445a664..86196f090ac9 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -13,9 +13,6 @@
 
 void msm_hdmi_bridge_destroy(struct drm_bridge *bridge)
 {
-   struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
-
-   msm_hdmi_hpd_disable(hdmi_bridge);
drm_bridge_remove(bridge);
 }
 
@@ -299,6 +296,9 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs 
= {
.mode_valid = msm_hdmi_bridge_mode_valid,
.get_edid = msm_hdmi_bridge_get_edid,
.detect = msm_hdmi_bridge_detect,
+
+   .hpd_enable = msm_hdmi_hpd_enable,
+   .hpd_disable = msm_hdmi_hpd_disable,
 };
 
 static void
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 2080e7c6700c..04d00b6f36fd 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -60,7 +60,7 @@ static void msm_hdmi_phy_reset(struct hdmi *hdmi)
}
 }
 
-int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
+void msm_hdmi_hpd_enable(struct drm_bridge *bridge)
 {
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
@@ -70,8 +70,8 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
unsigned long flags;
 
ret = pm_runtime_resume_and_get(dev);
-   if (ret)
-   return ret;
+   if (WARN_ON(ret))
+   return;
 
mutex_lock(&hdmi->state_mutex);
msm_hdmi_set_mode(hdmi, false);
@@ -99,12 +99,11 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
hdmi_write(hdmi, REG_HDMI_HPD_CTRL,
HDMI_HPD_CTRL_ENABLE | hpd_ctrl);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
-
-   return 0;
 }
 
-void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge)
+void msm_hdmi_hpd_disable(struct drm_bridge *bridge)
 {
+   struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
struct device *dev = &hdmi->pdev->dev;
 
-- 
2.39.2

[Freedreno] [PATCH 12/15] drm/msm/hdmi: expand the HDMI_CFG macro

2023-06-30 Thread Dmitry Baryshkov

Expand the HDMI_CFG() macro in HDMI config description. It has no added
value other than hiding some boilerplate declarations.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 16 
 drivers/gpu/drm/msm/hdmi/hdmi.h |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 621dcc981794..195e1ac90c5a 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -236,24 +236,24 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
  * The hdmi device:
  */
 
-#define HDMI_CFG(item, entry) \
-   .item ## _names = item ##_names_ ## entry, \
-   .item ## _cnt   = ARRAY_SIZE(item ## _names_ ## entry)
-
 static const char *pwr_reg_names_8960[] = {"core-vdda"};
 static const char *pwr_clk_names_8960[] = {"core", "master_iface", 
"slave_iface"};
 
 static const struct hdmi_platform_config hdmi_tx_8960_config = {
-   HDMI_CFG(pwr_reg, 8960),
-   HDMI_CFG(pwr_clk, 8960),
+   .pwr_reg_names = pwr_reg_names_8960,
+   .pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names_8960),
+   .pwr_clk_names = pwr_clk_names_8960,
+   .pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names_8960),
 };
 
 static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
 static const char *pwr_clk_names_8x74[] = {"iface", "core", "mdp_core", 
"alt_iface"};
 
 static const struct hdmi_platform_config hdmi_tx_8974_config = {
-   HDMI_CFG(pwr_reg, 8x74),
-   HDMI_CFG(pwr_clk, 8x74),
+   .pwr_reg_names = pwr_reg_names_8x74,
+   .pwr_reg_cnt = ARRAY_SIZE(pwr_reg_names_8x74),
+   .pwr_clk_names = pwr_clk_names_8x74,
+   .pwr_clk_cnt = ARRAY_SIZE(pwr_clk_names_8x74),
 };
 
 /*
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 22c91c17539c..acfd99d2876a 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -89,7 +89,7 @@ struct hdmi_platform_config {
const char **pwr_reg_names;
int pwr_reg_cnt;
 
-   /* clks that need to be on for hpd: */
+   /* clks that need to be on: */
const char **pwr_clk_names;
int pwr_clk_cnt;
 };
-- 
2.39.2

[Freedreno] [PATCH 04/15] drm/msm/hdmi: switch to atomic_pre_enable/post_disable

2023-06-30 Thread Dmitry Baryshkov

In preparation of reworking the HDMI mode setting, switch pre_enable and
post_disable callbacks to their atomic variants.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index fbcf4dd91cd9..f9293f7d8f34 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -128,7 +128,8 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi)
hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val);
 }
 
-static void msm_hdmi_bridge_pre_enable(struct drm_bridge *bridge)
+static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state 
*old_bridge_state)
 {
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
@@ -154,7 +155,8 @@ static void msm_hdmi_bridge_pre_enable(struct drm_bridge 
*bridge)
msm_hdmi_hdcp_on(hdmi->hdcp_ctrl);
 }
 
-static void msm_hdmi_bridge_post_disable(struct drm_bridge *bridge)
+static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge,
+   struct drm_bridge_state 
*old_bridge_state)
 {
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
@@ -291,8 +293,13 @@ static enum drm_mode_status 
msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge
 }
 
 static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = {
-   .pre_enable = msm_hdmi_bridge_pre_enable,
-   .post_disable = msm_hdmi_bridge_post_disable,
+   .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+   .atomic_reset = drm_atomic_helper_bridge_reset,
+
+   .atomic_pre_enable = msm_hdmi_bridge_atomic_pre_enable,
+   .atomic_post_disable = msm_hdmi_bridge_atomic_post_disable,
+
.mode_set = msm_hdmi_bridge_mode_set,
.mode_valid = msm_hdmi_bridge_mode_valid,
.get_edid = msm_hdmi_bridge_get_edid,
-- 
2.39.2

[Freedreno] [PATCH 11/15] drm/msm/hdmi: rename hpd_clks to pwr_clks

2023-06-30 Thread Dmitry Baryshkov

As these clocks are now used in the runtime PM callbacks, they have no
connection to 'HPD'. Rename corresponding fields to follow clocks
purpose, to power up the HDMI controller.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 26 +-
 drivers/gpu/drm/msm/hdmi/hdmi.h |  6 +++---
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 9aba6a67816a..621dcc981794 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -241,19 +241,19 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
.item ## _cnt   = ARRAY_SIZE(item ## _names_ ## entry)
 
 static const char *pwr_reg_names_8960[] = {"core-vdda"};
-static const char *hpd_clk_names_8960[] = {"core", "master_iface", 
"slave_iface"};
+static const char *pwr_clk_names_8960[] = {"core", "master_iface", 
"slave_iface"};
 
 static const struct hdmi_platform_config hdmi_tx_8960_config = {
HDMI_CFG(pwr_reg, 8960),
-   HDMI_CFG(hpd_clk, 8960),
+   HDMI_CFG(pwr_clk, 8960),
 };
 
 static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
-static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core", 
"alt_iface"};
+static const char *pwr_clk_names_8x74[] = {"iface", "core", "mdp_core", 
"alt_iface"};
 
 static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
-   HDMI_CFG(hpd_clk, 8x74),
+   HDMI_CFG(pwr_clk, 8x74),
 };
 
 /*
@@ -459,17 +459,17 @@ static int msm_hdmi_dev_probe(struct platform_device 
*pdev)
if (ret)
return dev_err_probe(dev, ret, "failed to get pwr 
regulators\n");
 
-   hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
- config->hpd_clk_cnt,
- sizeof(hdmi->hpd_clks[0]),
+   hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
+ config->pwr_clk_cnt,
+ sizeof(hdmi->pwr_clks[0]),
  GFP_KERNEL);
-   if (!hdmi->hpd_clks)
+   if (!hdmi->pwr_clks)
return -ENOMEM;
 
-   for (i = 0; i < config->hpd_clk_cnt; i++)
-   hdmi->hpd_clks[i].id = config->hpd_clk_names[i];
+   for (i = 0; i < config->pwr_clk_cnt; i++)
+   hdmi->pwr_clks[i].id = config->pwr_clk_names[i];
 
-   ret = devm_clk_bulk_get(&pdev->dev, config->hpd_clk_cnt, 
hdmi->hpd_clks);
+   ret = devm_clk_bulk_get(&pdev->dev, config->pwr_clk_cnt, 
hdmi->pwr_clks);
if (ret)
return ret;
 
@@ -529,7 +529,7 @@ static int msm_hdmi_runtime_suspend(struct device *dev)
struct hdmi *hdmi = dev_get_drvdata(dev);
const struct hdmi_platform_config *config = hdmi->config;
 
-   clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
+   clk_bulk_disable_unprepare(config->pwr_clk_cnt, hdmi->pwr_clks);
 
pinctrl_pm_select_sleep_state(dev);
 
@@ -552,7 +552,7 @@ static int msm_hdmi_runtime_resume(struct device *dev)
if (ret)
goto fail;
 
-   ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+   ret = clk_bulk_prepare_enable(config->pwr_clk_cnt, hdmi->pwr_clks);
if (ret)
goto fail;
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 896f4b3d5978..22c91c17539c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -49,7 +49,7 @@ struct hdmi {
phys_addr_t mmio_phy_addr;
 
struct regulator_bulk_data *pwr_regs;
-   struct clk_bulk_data *hpd_clks;
+   struct clk_bulk_data *pwr_clks;
struct clk *extp_clk;
 
struct gpio_desc *hpd_gpiod;
@@ -90,8 +90,8 @@ struct hdmi_platform_config {
int pwr_reg_cnt;
 
/* clks that need to be on for hpd: */
-   const char **hpd_clk_names;
-   int hpd_clk_cnt;
+   const char **pwr_clk_names;
+   int pwr_clk_cnt;
 };
 
 struct hdmi_bridge {
-- 
2.39.2

[Freedreno] [PATCH 05/15] drm/msm/hdmi: set infoframes on all pre_enable calls

2023-06-30 Thread Dmitry Baryshkov

In consequent modeset calls, the atomic_pre_enable() will be called
several times without calling atomic_post_disable() inbetween. Thus
iframes will not be updated for the next mode. Fix this by setting the
iframe outside of the !power_on check.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index f9293f7d8f34..bb10b35194ff 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -141,10 +141,11 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct 
drm_bridge *bridge,
msm_hdmi_phy_resource_enable(phy);
msm_hdmi_power_on(bridge);
hdmi->power_on = true;
-   if (hdmi->hdmi_mode) {
-   msm_hdmi_config_avi_infoframe(hdmi);
-   msm_hdmi_audio_update(hdmi);
-   }
+   }
+
+   if (hdmi->hdmi_mode) {
+   msm_hdmi_config_avi_infoframe(hdmi);
+   msm_hdmi_audio_update(hdmi);
}
 
msm_hdmi_phy_powerup(phy, hdmi->pixclock);
-- 
2.39.2

[Freedreno] [PATCH 07/15] drm/msm/hdmi: switch to clk_bulk API

2023-06-30 Thread Dmitry Baryshkov

The last platform using legacy clock names for HDMI block (APQ8064)
switched to new clock names in 5.16. It's time to stop caring about old
DT, drop hand-coded helpers and switch to clk_bulk_* API.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 15 ---
 drivers/gpu/drm/msm/hdmi/hdmi.h |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 39 ++---
 3 files changed, 19 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index e05bfc2ab64c..78226e6c6e90 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -479,17 +479,12 @@ static int msm_hdmi_dev_probe(struct platform_device 
*pdev)
if (!hdmi->hpd_clks)
return -ENOMEM;
 
-   for (i = 0; i < config->hpd_clk_cnt; i++) {
-   struct clk *clk;
+   for (i = 0; i < config->hpd_clk_cnt; i++)
+   hdmi->hpd_clks[i].id = config->hpd_clk_names[i];
 
-   clk = msm_clk_get(pdev, config->hpd_clk_names[i]);
-   if (IS_ERR(clk))
-   return dev_err_probe(dev, PTR_ERR(clk),
-"failed to get hpd clk: %s\n",
-config->hpd_clk_names[i]);
-
-   hdmi->hpd_clks[i] = clk;
-   }
+   ret = devm_clk_bulk_get(&pdev->dev, config->hpd_clk_cnt, 
hdmi->hpd_clks);
+   if (ret)
+   return ret;
 
hdmi->extp_clk = devm_clk_get_optional(&pdev->dev, "extp");
if (IS_ERR(hdmi->extp_clk))
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 4e32281296ca..48052466f856 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -50,7 +50,7 @@ struct hdmi {
 
struct regulator_bulk_data *hpd_regs;
struct regulator_bulk_data *pwr_regs;
-   struct clk **hpd_clks;
+   struct clk_bulk_data *hpd_clks;
struct clk *extp_clk;
 
struct gpio_desc *hpd_gpiod;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 6b017a07b57f..3d3d72ff6a83 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -60,27 +60,6 @@ static void msm_hdmi_phy_reset(struct hdmi *hdmi)
}
 }
 
-static void enable_hpd_clocks(struct hdmi *hdmi, bool enable)
-{
-   const struct hdmi_platform_config *config = hdmi->config;
-   struct device *dev = &hdmi->pdev->dev;
-   int i, ret;
-
-   if (enable) {
-   for (i = 0; i < config->hpd_clk_cnt; i++) {
-   ret = clk_prepare_enable(hdmi->hpd_clks[i]);
-   if (ret) {
-   DRM_DEV_ERROR(dev,
-   "failed to enable hpd clk: %s (%d)\n",
-   config->hpd_clk_names[i], ret);
-   }
-   }
-   } else {
-   for (i = config->hpd_clk_cnt - 1; i >= 0; i--)
-   clk_disable_unprepare(hdmi->hpd_clks[i]);
-   }
-}
-
 int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
 {
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
@@ -107,7 +86,9 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
 
pm_runtime_get_sync(dev);
-   enable_hpd_clocks(hdmi, true);
+   ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+   if (ret)
+   goto fail;
 
msm_hdmi_set_mode(hdmi, false);
msm_hdmi_phy_reset(hdmi);
@@ -150,7 +131,7 @@ void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge)
 
msm_hdmi_set_mode(hdmi, false);
 
-   enable_hpd_clocks(hdmi, false);
+   clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
pm_runtime_put(dev);
 
ret = pinctrl_pm_select_sleep_state(dev);
@@ -194,14 +175,20 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
 
 static enum drm_connector_status detect_reg(struct hdmi *hdmi)
 {
-   uint32_t hpd_int_status;
+   const struct hdmi_platform_config *config = hdmi->config;
+   uint32_t hpd_int_status = 0;
+   int ret;
 
pm_runtime_get_sync(&hdmi->pdev->dev);
-   enable_hpd_clocks(hdmi, true);
+   ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+   if (ret)
+   goto out;
 
hpd_int_status = hdmi_read(hdmi, REG_HDMI_HPD_INT_STATUS);
 
-   enable_hpd_clocks(hdmi, false);
+   clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
+
+out:
pm_runtime_put(&hdmi->pdev->dev);
 
return (hpd_int_status & HDMI_HPD_INT_STATUS_CABLE_DETECTED) ?
-- 
2.39.2

[Freedreno] [PATCH 13/15] drm/msm/hdmi: drop hpd-gpios support

2023-06-30 Thread Dmitry Baryshkov

Supporting simultaneous check of native HPD and the external GPIO proved
to be less stable than just native HPD. Drop the hpd-gpios support,
leaving just the native HPD support. In case the native HPD doesn't work
the user is urged to switch to specifying the HPD property to the
hdmi-connector device.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 14 ++--
 drivers/gpu/drm/msm/hdmi/hdmi.h |  2 --
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 53 +++--
 3 files changed, 7 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 195e1ac90c5a..598c8284f125 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -478,17 +478,9 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
 "failed to get extp clock\n");
 
-   hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
-   /* This will catch e.g. -EPROBE_DEFER */
-   if (IS_ERR(hdmi->hpd_gpiod))
-   return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
-"failed to get hpd gpio\n");
-
-   if (!hdmi->hpd_gpiod)
-   DBG("failed to get HPD gpio");
-
-   if (hdmi->hpd_gpiod)
-   gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
+   if (of_find_property(dev->of_node, "hpd-gpios", NULL) ||
+   of_find_property(dev->of_node, "hpd-gpio", NULL))
+   dev_warn(dev, "hpd-gpios is not supported anymore, please 
migrate to the hdmi-connector\n");
 
ret = msm_hdmi_get_phy(hdmi);
if (ret) {
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index acfd99d2876a..252d617939d4 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -52,8 +52,6 @@ struct hdmi {
struct clk_bulk_data *pwr_clks;
struct clk *extp_clk;
 
-   struct gpio_desc *hpd_gpiod;
-
struct hdmi_phy *phy;
struct device *phy_dev;
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 4167ba5a3b03..f3d6cc184999 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -69,9 +69,6 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
int ret;
unsigned long flags;
 
-   if (hdmi->hpd_gpiod)
-   gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
-
ret = pm_runtime_resume_and_get(dev);
if (ret)
return ret;
@@ -145,8 +142,11 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
}
 }
 
-static enum drm_connector_status detect_reg(struct hdmi *hdmi)
+enum drm_connector_status msm_hdmi_bridge_detect(
+   struct drm_bridge *bridge)
 {
+   struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
+   struct hdmi *hdmi = hdmi_bridge->hdmi;
uint32_t hpd_int_status = 0;
int ret;
 
@@ -162,48 +162,3 @@ static enum drm_connector_status detect_reg(struct hdmi 
*hdmi)
return (hpd_int_status & HDMI_HPD_INT_STATUS_CABLE_DETECTED) ?
connector_status_connected : 
connector_status_disconnected;
 }
-
-#define HPD_GPIO_INDEX 2
-static enum drm_connector_status detect_gpio(struct hdmi *hdmi)
-{
-   return gpiod_get_value(hdmi->hpd_gpiod) ?
-   connector_status_connected :
-   connector_status_disconnected;
-}
-
-enum drm_connector_status msm_hdmi_bridge_detect(
-   struct drm_bridge *bridge)
-{
-   struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
-   struct hdmi *hdmi = hdmi_bridge->hdmi;
-   enum drm_connector_status stat_gpio, stat_reg;
-   int retry = 20;
-
-   /*
-* some platforms may not have hpd gpio. Rely only on the status
-* provided by REG_HDMI_HPD_INT_STATUS in this case.
-*/
-   if (!hdmi->hpd_gpiod)
-   return detect_reg(hdmi);
-
-   do {
-   stat_gpio = detect_gpio(hdmi);
-   stat_reg  = detect_reg(hdmi);
-
-   if (stat_gpio == stat_reg)
-   break;
-
-   mdelay(10);
-   } while (--retry);
-
-   /* the status we get from reading gpio seems to be more reliable,
-* so trust that one the most if we didn't manage to get hdmi and
-* gpio status to agree:
-*/
-   if (stat_gpio != stat_reg) {
-   DBG("HDMI_HPD_INT_STATUS tells us: %d", stat_reg);
-   DBG("hpd gpio tells us: %d", stat_gpio);
-   }
-
-   return stat_gpio;
-}
-- 
2.39.2

[Freedreno] [PATCH 10/15] drm/msm/hdmi: implement proper runtime PM handling

2023-06-30 Thread Dmitry Baryshkov

It is completely not obvious, but the so-called 'hpd' clocks and
regulators are required for the HDMI host to function properly. Merge
pwr and hpd regulators. Use regulators, clocks and pinctrl to implement
proper runtime PM callbacks.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c| 62 +++---
 drivers/gpu/drm/msm/hdmi/hdmi.h|  7 +--
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 12 -
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 42 +
 4 files changed, 48 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 78226e6c6e90..9aba6a67816a 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -7,6 +7,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -239,11 +240,11 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
.item ## _names = item ##_names_ ## entry, \
.item ## _cnt   = ARRAY_SIZE(item ## _names_ ## entry)
 
-static const char *hpd_reg_names_8960[] = {"core-vdda"};
+static const char *pwr_reg_names_8960[] = {"core-vdda"};
 static const char *hpd_clk_names_8960[] = {"core", "master_iface", 
"slave_iface"};
 
 static const struct hdmi_platform_config hdmi_tx_8960_config = {
-   HDMI_CFG(hpd_reg, 8960),
+   HDMI_CFG(pwr_reg, 8960),
HDMI_CFG(hpd_clk, 8960),
 };
 
@@ -444,20 +445,6 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
if (hdmi->irq < 0)
return hdmi->irq;
 
-   hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
- config->hpd_reg_cnt,
- sizeof(hdmi->hpd_regs[0]),
- GFP_KERNEL);
-   if (!hdmi->hpd_regs)
-   return -ENOMEM;
-
-   for (i = 0; i < config->hpd_reg_cnt; i++)
-   hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
-
-   ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt, 
hdmi->hpd_regs);
-   if (ret)
-   return dev_err_probe(dev, ret, "failed to get hpd 
regulators\n");
-
hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
  config->pwr_reg_cnt,
  sizeof(hdmi->pwr_regs[0]),
@@ -537,6 +524,48 @@ static int msm_hdmi_dev_remove(struct platform_device 
*pdev)
return 0;
 }
 
+static int msm_hdmi_runtime_suspend(struct device *dev)
+{
+   struct hdmi *hdmi = dev_get_drvdata(dev);
+   const struct hdmi_platform_config *config = hdmi->config;
+
+   clk_bulk_disable_unprepare(config->hpd_clk_cnt, hdmi->hpd_clks);
+
+   pinctrl_pm_select_sleep_state(dev);
+
+   regulator_bulk_disable(config->pwr_reg_cnt, hdmi->pwr_regs);
+
+   return 0;
+}
+
+static int msm_hdmi_runtime_resume(struct device *dev)
+{
+   struct hdmi *hdmi = dev_get_drvdata(dev);
+   const struct hdmi_platform_config *config = hdmi->config;
+   int ret;
+
+   ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
+   if (ret)
+   return ret;
+
+   ret = pinctrl_pm_select_default_state(dev);
+   if (ret)
+   goto fail;
+
+   ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
+   if (ret)
+   goto fail;
+
+   return 0;
+
+fail:
+   pinctrl_pm_select_sleep_state(dev);
+
+   return ret;
+}
+
+DEFINE_RUNTIME_DEV_PM_OPS(msm_hdmi_pm_ops, msm_hdmi_runtime_suspend, 
msm_hdmi_runtime_resume, NULL);
+
 static const struct of_device_id msm_hdmi_dt_match[] = {
{ .compatible = "qcom,hdmi-tx-8996", .data = &hdmi_tx_8974_config },
{ .compatible = "qcom,hdmi-tx-8994", .data = &hdmi_tx_8974_config },
@@ -553,6 +582,7 @@ static struct platform_driver msm_hdmi_driver = {
.driver = {
.name = "hdmi_msm",
.of_match_table = msm_hdmi_dt_match,
+   .pm = &msm_hdmi_pm_ops,
},
 };
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 48052466f856..896f4b3d5978 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -48,7 +48,6 @@ struct hdmi {
void __iomem *qfprom_mmio;
phys_addr_t mmio_phy_addr;
 
-   struct regulator_bulk_data *hpd_regs;
struct regulator_bulk_data *pwr_regs;
struct clk_bulk_data *hpd_clks;
struct clk *extp_clk;
@@ -86,11 +85,7 @@ struct hdmi {
 
 /* platform config data (ie. from DT, or pdata) */
 struct hdmi_platform_config {
-   /* regulators that need to be on for hpd: */
-   const char **hpd_reg_names;
-   int hpd_reg_cnt;
-
-   /* regulators that need to be on for screen pwr: */
+   /* regulators that need to be on: */
const char **pwr_reg_names;
int pwr_reg_cnt;
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 4aa11eabbf2a..d6d5

[Freedreno] [PATCH 06/15] drm/msm/hdmi: drop clock frequency assignment

2023-06-30 Thread Dmitry Baryshkov

The only clock which has frequency being set through hpd_freqs is the
"core" aka MDSS_HDMI_CLK clock. It always has the specified frequency,
so we can drop corresponding clk_set_rate() call together with the
hpd_freq infrastructure.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 2 --
 drivers/gpu/drm/msm/hdmi/hdmi.h | 1 -
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 9 -
 3 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index a2780aba6d3c..e05bfc2ab64c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -249,12 +249,10 @@ static const struct hdmi_platform_config 
hdmi_tx_8960_config = {
 
 static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
 static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core", 
"alt_iface"};
-static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0, 0};
 
 static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
HDMI_CFG(hpd_clk, 8x74),
-   .hpd_freq  = hpd_clk_freq_8x74,
 };
 
 /*
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 2d405da63bd0..4e32281296ca 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -96,7 +96,6 @@ struct hdmi_platform_config {
 
/* clks that need to be on for hpd: */
const char **hpd_clk_names;
-   const long unsigned *hpd_freq;
int hpd_clk_cnt;
 };
 
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index bfa827b47989..6b017a07b57f 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -68,15 +68,6 @@ static void enable_hpd_clocks(struct hdmi *hdmi, bool enable)
 
if (enable) {
for (i = 0; i < config->hpd_clk_cnt; i++) {
-   if (config->hpd_freq && config->hpd_freq[i]) {
-   ret = clk_set_rate(hdmi->hpd_clks[i],
-  config->hpd_freq[i]);
-   if (ret)
-   dev_warn(dev,
-"failed to set clk %s (%d)\n",
-config->hpd_clk_names[i], ret);
-   }
-
ret = clk_prepare_enable(hdmi->hpd_clks[i]);
if (ret) {
DRM_DEV_ERROR(dev,
-- 
2.39.2

[Freedreno] [PATCH 08/15] drm/msm/hdmi: switch to pm_runtime_resume_and_get()

2023-06-30 Thread Dmitry Baryshkov

The pm_runtime_get_sync() function is a bad choise for runtime power
management. Switch HDMI driver to pm_runtime_resume_and_get() and add
proper error handling, while we are at it.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 12 ++--
 drivers/gpu/drm/msm/hdmi/hdmi_phy.c|  6 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index bb10b35194ff..4aa11eabbf2a 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -27,7 +27,7 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
const struct hdmi_platform_config *config = hdmi->config;
int ret;
 
-   pm_runtime_get_sync(&hdmi->pdev->dev);
+   pm_runtime_resume_and_get(&hdmi->pdev->dev);
 
ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
if (ret)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 3d3d72ff6a83..7de538046a52 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -85,7 +85,12 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
if (hdmi->hpd_gpiod)
gpiod_set_value_cansleep(hdmi->hpd_gpiod, 1);
 
-   pm_runtime_get_sync(dev);
+   ret = pm_runtime_resume_and_get(dev);
+   if (ret) {
+   DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
+   goto fail;
+   }
+
ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
if (ret)
goto fail;
@@ -179,7 +184,10 @@ static enum drm_connector_status detect_reg(struct hdmi 
*hdmi)
uint32_t hpd_int_status = 0;
int ret;
 
-   pm_runtime_get_sync(&hdmi->pdev->dev);
+   ret = pm_runtime_resume_and_get(&hdmi->pdev->dev);
+   if (ret)
+   goto out;
+
ret = clk_bulk_prepare_enable(config->hpd_clk_cnt, hdmi->hpd_clks);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
index 9780107e1cc9..d1f6b53c3109 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
@@ -57,7 +57,11 @@ int msm_hdmi_phy_resource_enable(struct hdmi_phy *phy)
struct device *dev = &phy->pdev->dev;
int i, ret = 0;
 
-   pm_runtime_get_sync(dev);
+   ret = pm_runtime_resume_and_get(dev);
+   if (ret) {
+   DRM_DEV_ERROR(dev, "runtime resume failed: %d\n", ret);
+   return ret;
+   }
 
ret = regulator_bulk_enable(cfg->num_regs, phy->regs);
if (ret) {
-- 
2.39.2

[Freedreno] [PATCH 02/15] drm/msm/hdmi: simplify extp clock handling

2023-06-30 Thread Dmitry Baryshkov

With the extp being the only "power" clock left, remove the surrounding
loops and handle the extp clock directly.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c| 24 ---
 drivers/gpu/drm/msm/hdmi/hdmi.h|  6 +
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 33 ++
 3 files changed, 18 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 0fc3df43aa70..a2780aba6d3c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -248,13 +248,11 @@ static const struct hdmi_platform_config 
hdmi_tx_8960_config = {
 };
 
 static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
-static const char *pwr_clk_names_8x74[] = {"extp"};
 static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core", 
"alt_iface"};
 static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0, 0};
 
 static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
-   HDMI_CFG(pwr_clk, 8x74),
HDMI_CFG(hpd_clk, 8x74),
.hpd_freq  = hpd_clk_freq_8x74,
 };
@@ -495,24 +493,10 @@ static int msm_hdmi_dev_probe(struct platform_device 
*pdev)
hdmi->hpd_clks[i] = clk;
}
 
-   hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
- config->pwr_clk_cnt,
- sizeof(hdmi->pwr_clks[0]),
- GFP_KERNEL);
-   if (!hdmi->pwr_clks)
-   return -ENOMEM;
-
-   for (i = 0; i < config->pwr_clk_cnt; i++) {
-   struct clk *clk;
-
-   clk = msm_clk_get(pdev, config->pwr_clk_names[i]);
-   if (IS_ERR(clk))
-   return dev_err_probe(dev, PTR_ERR(clk),
-"failed to get pwr clk: %s\n",
-config->pwr_clk_names[i]);
-
-   hdmi->pwr_clks[i] = clk;
-   }
+   hdmi->extp_clk = devm_clk_get_optional(&pdev->dev, "extp");
+   if (IS_ERR(hdmi->extp_clk))
+   return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
+"failed to get extp clock\n");
 
hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
/* This will catch e.g. -EPROBE_DEFER */
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index e8dbee50637f..2d405da63bd0 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -51,7 +51,7 @@ struct hdmi {
struct regulator_bulk_data *hpd_regs;
struct regulator_bulk_data *pwr_regs;
struct clk **hpd_clks;
-   struct clk **pwr_clks;
+   struct clk *extp_clk;
 
struct gpio_desc *hpd_gpiod;
 
@@ -98,10 +98,6 @@ struct hdmi_platform_config {
const char **hpd_clk_names;
const long unsigned *hpd_freq;
int hpd_clk_cnt;
-
-   /* clks that need to be on for screen pwr (ie pixel clk): */
-   const char **pwr_clk_names;
-   int pwr_clk_cnt;
 };
 
 struct hdmi_bridge {
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 9b1391d27ed3..62ce1455f974 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -25,7 +25,7 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
const struct hdmi_platform_config *config = hdmi->config;
-   int i, ret;
+   int ret;
 
pm_runtime_get_sync(&hdmi->pdev->dev);
 
@@ -33,21 +33,15 @@ static void msm_hdmi_power_on(struct drm_bridge *bridge)
if (ret)
DRM_DEV_ERROR(dev->dev, "failed to enable pwr regulator: %d\n", 
ret);
 
-   if (config->pwr_clk_cnt > 0) {
+   if (hdmi->extp_clk) {
DBG("pixclock: %lu", hdmi->pixclock);
-   ret = clk_set_rate(hdmi->pwr_clks[0], hdmi->pixclock);
-   if (ret) {
-   DRM_DEV_ERROR(dev->dev, "failed to set pixel clk: %s 
(%d)\n",
-   config->pwr_clk_names[0], ret);
-   }
-   }
+   ret = clk_set_rate(hdmi->extp_clk, hdmi->pixclock);
+   if (ret)
+   DRM_DEV_ERROR(dev->dev, "failed to set extp clk rate: 
%d\n", ret);
 
-   for (i = 0; i < config->pwr_clk_cnt; i++) {
-   ret = clk_prepare_enable(hdmi->pwr_clks[i]);
-   if (ret) {
-   DRM_DEV_ERROR(dev->dev, "failed to enable pwr clk: %s 
(%d)\n",
-   config->pwr_clk_names[i], ret);
-   }
+   ret = clk_prepare_enable(hdmi->extp_clk);
+   if (ret)
+   DRM_DEV_ERROR(dev-

[Freedreno] [PATCH 03/15] drm/msm/hdmi: correct indentation of HDMI bridge functions

2023-06-30 Thread Dmitry Baryshkov

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c 
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 62ce1455f974..fbcf4dd91cd9 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -291,12 +291,12 @@ static enum drm_mode_status 
msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge
 }
 
 static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = {
-   .pre_enable = msm_hdmi_bridge_pre_enable,
-   .post_disable = msm_hdmi_bridge_post_disable,
-   .mode_set = msm_hdmi_bridge_mode_set,
-   .mode_valid = msm_hdmi_bridge_mode_valid,
-   .get_edid = msm_hdmi_bridge_get_edid,
-   .detect = msm_hdmi_bridge_detect,
+   .pre_enable = msm_hdmi_bridge_pre_enable,
+   .post_disable = msm_hdmi_bridge_post_disable,
+   .mode_set = msm_hdmi_bridge_mode_set,
+   .mode_valid = msm_hdmi_bridge_mode_valid,
+   .get_edid = msm_hdmi_bridge_get_edid,
+   .detect = msm_hdmi_bridge_detect,
 };
 
 static void
-- 
2.39.2

[Freedreno] [PATCH 01/15] drm/msm/hdmi: move the alt_iface clock to the hpd list

2023-06-30 Thread Dmitry Baryshkov

According to the vendor kernel [1] , the alt_iface clock should be
enabled together with the rest of HPD clocks, to make HPD to work
properly.

[1] 
https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/e07a5487e521e57f76083c0a6e2f995414ac6d03

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 3132105a2a43..0fc3df43aa70 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -248,9 +248,9 @@ static const struct hdmi_platform_config 
hdmi_tx_8960_config = {
 };
 
 static const char *pwr_reg_names_8x74[] = {"core-vdda", "core-vcc"};
-static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"};
-static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"};
-static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0};
+static const char *pwr_clk_names_8x74[] = {"extp"};
+static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core", 
"alt_iface"};
+static unsigned long hpd_clk_freq_8x74[] = {0, 1920, 0, 0};
 
 static const struct hdmi_platform_config hdmi_tx_8974_config = {
HDMI_CFG(pwr_reg, 8x74),
-- 
2.39.2

[Freedreno] [PATCH 00/15] drm/msm/hdmi: rework and fix the HPD even generation

2023-06-30 Thread Dmitry Baryshkov

The MSM HDMI driver is plagued with the long-standing bug. If HDMI cable
is disconnected, in most of the cases cable reconnection will not be
detected properly. We have been carrying the patch from [1] in our
integration tree for ages. The time has come to fix the long-standing
bug and implement proper HPD handling.

This series was tested on msm8996 and apq8064 boards. On APQ8064 in some
rare cases I get the backtrace logged at [2]. It is unclear if it is a
result of this series or not. I'll investigate it further later on.

[1] 
https://lore.kernel.org/linux-arm-msm/20171027105732.19235-2-arch...@codeaurora.org/
[2] https://gitlab.freedesktop.org/drm/msm/-/issues/27

Dmitry Baryshkov (15):
  drm/msm/hdmi: move the alt_iface clock to the hpd list
  drm/msm/hdmi: simplify extp clock handling
  drm/msm/hdmi: correct indentation of HDMI bridge functions
  drm/msm/hdmi: switch to atomic_pre_enable/post_disable
  drm/msm/hdmi: set infoframes on all pre_enable calls
  drm/msm/hdmi: drop clock frequency assignment
  drm/msm/hdmi: switch to clk_bulk API
  drm/msm/hdmi: switch to pm_runtime_resume_and_get()
  drm/msm/hdmi: add runtime PM calls to DDC transfer function
  drm/msm/hdmi: implement proper runtime PM handling
  drm/msm/hdmi: rename hpd_clks to pwr_clks
  drm/msm/hdmi: expand the HDMI_CFG macro
  drm/msm/hdmi: drop hpd-gpios support
  drm/msm/hdmi: ensure that HDMI is one if HPD is requested
  drm/msm/hdmi: wire in hpd_enable/hpd_disable bridge ops

 drivers/gpu/drm/msm/hdmi/hdmi.c| 142 -
 drivers/gpu/drm/msm/hdmi/hdmi.h|  26 ++---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c |  91 
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c| 141 +---
 drivers/gpu/drm/msm/hdmi/hdmi_i2c.c|  14 ++-
 drivers/gpu/drm/msm/hdmi/hdmi_phy.c|   6 +-
 6 files changed, 160 insertions(+), 260 deletions(-)

-- 
2.39.2

Re: [Freedreno] [PATCH] drm/msm/adreno: Assign revn to A635

2023-06-30 Thread Rob Clark

On Wed, Jun 28, 2023 at 12:54 PM Dmitry Baryshkov
 wrote:
>
> On 28/06/2023 22:05, Konrad Dybcio wrote:
> > Recently, a WARN_ON() was introduced to ensure that revn is filled before
> > adreno_is_aXYZ is called. This however doesn't work very well when revn is
> > 0 by design (such as for A635). Fill it in as a stopgap solution for
> > -fixes.
> >
> > Signed-off-by: Konrad Dybcio 
>
> Reviewed-by: Dmitry Baryshkov 
>
> I'd have probably added:
>
> Fixes: cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified
> before being set")
>
> or
>
> Fixes: 192f4ee3e408 ("drm/msm/a6xx: Add support for Adreno 7c Gen 3 gpu")

I'd lean towards the former, given that this is a temporary workaround
until we do a more comprehensive overhaul and remove revn entirely

BR,
-R

>
>
> > ---
> >   drivers/gpu/drm/msm/adreno/adreno_device.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c 
> > b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > index cb94cfd137a8..8ea7eae9fc52 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > @@ -345,6 +345,7 @@ static const struct adreno_info gpulist[] = {
> >   .address_space_size = SZ_16G,
> >   }, {
> >   .rev = ADRENO_REV(6, 3, 5, ANY_ID),
> > + .revn = 635,
> >   .fw = {
> >   [ADRENO_FW_SQE] = "a660_sqe.fw",
> >   [ADRENO_FW_GMU] = "a660_gmu.bin",
> >
> > ---
> > base-commit: 5c875096d59010cee4e00da1f9c7bdb07a025dc2
> > change-id: 20230628-topic-a635-1b3c2c987417
> >
> > Best regards,
>
> --
> With best wishes
> Dmitry
>

Re: [Freedreno] [PATCH RFC v4 3/7] drm/atomic: Move framebuffer checks to helper

2023-06-30 Thread Jessica Zhang





On 6/29/2023 5:43 PM, Dmitry Baryshkov wrote:

On 30/06/2023 03:25, Jessica Zhang wrote:

Currently framebuffer checks happen directly in
drm_atomic_plane_check(). Move these checks into their own helper
method.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic.c | 130 
---

  1 file changed, 74 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index b4c6ffc438da..404b984d2d9f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -580,6 +580,76 @@ plane_switching_crtc(const struct drm_plane_state 
*old_plane_state,

  return true;
  }
+static int drm_atomic_check_fb(const struct drm_plane_state *state)
+{
+    struct drm_plane *plane = state->plane;
+    const struct drm_framebuffer *fb = state->fb;
+    struct drm_mode_rect *clips;
+
+    uint32_t num_clips;
+    unsigned int fb_width, fb_height;
+    int ret;
+
+    /* Check whether this plane supports the fb pixel format. */
+    ret = drm_plane_check_pixel_format(plane, fb->format->format,
+   fb->modifier);
+
+    if (ret) {
+    drm_dbg_atomic(plane->dev,
+   "[PLANE:%d:%s] invalid pixel format %p4cc, 
modifier 0x%llx\n",

+   plane->base.id, plane->name,
+   &fb->format->format, fb->modifier);
+    return ret;
+    }
+
+    fb_width = fb->width << 16;
+    fb_height = fb->height << 16;
+
+    /* Make sure source coordinates are inside the fb. */
+    if (state->src_w > fb_width ||
+    state->src_x > fb_width - state->src_w ||
+    state->src_h > fb_height ||
+    state->src_y > fb_height - state->src_h) {
+    drm_dbg_atomic(plane->dev,
+   "[PLANE:%d:%s] invalid source coordinates "
+   "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",
+   plane->base.id, plane->name,
+   state->src_w >> 16,
+   ((state->src_w & 0x) * 15625) >> 10,
+   state->src_h >> 16,
+   ((state->src_h & 0x) * 15625) >> 10,
+   state->src_x >> 16,
+   ((state->src_x & 0x) * 15625) >> 10,
+   state->src_y >> 16,
+   ((state->src_y & 0x) * 15625) >> 10,
+   fb->width, fb->height);
+    return -ENOSPC;
+    }
+
+    clips = __drm_plane_get_damage_clips(state);
+    num_clips = drm_plane_get_damage_clips_count(state);
+
+    /* Make sure damage clips are valid and inside the fb. */
+    while (num_clips > 0) {
+    if (clips->x1 >= clips->x2 ||
+    clips->y1 >= clips->y2 ||
+    clips->x1 < 0 ||
+    clips->y1 < 0 ||
+    clips->x2 > fb_width ||
+    clips->y2 > fb_height) {
+    drm_dbg_atomic(plane->dev,
+   "[PLANE:%d:%s] invalid damage clip %d %d %d 
%d\n",

+   plane->base.id, plane->name, clips->x1,
+   clips->y1, clips->x2, clips->y2);
+    return -EINVAL;
+    }
+    clips++;
+    num_clips--;
+    }
+
+    return 0;
+}
+
  /**
   * drm_atomic_plane_check - check plane state
   * @old_plane_state: old plane state to check
@@ -596,9 +666,6 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  struct drm_plane *plane = new_plane_state->plane;
  struct drm_crtc *crtc = new_plane_state->crtc;
  const struct drm_framebuffer *fb = new_plane_state->fb;
-    unsigned int fb_width, fb_height;
-    struct drm_mode_rect *clips;
-    uint32_t num_clips;
  int ret;
  /* either *both* CRTC and FB must be set, or neither */
@@ -625,17 +692,6 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  return -EINVAL;
  }
-    /* Check whether this plane supports the fb pixel format. */
-    ret = drm_plane_check_pixel_format(plane, fb->format->format,
-   fb->modifier);
-    if (ret) {
-    drm_dbg_atomic(plane->dev,
-   "[PLANE:%d:%s] invalid pixel format %p4cc, 
modifier 0x%llx\n",

-   plane->base.id, plane->name,
-   &fb->format->format, fb->modifier);
-    return ret;
-    }
-
  /* Give drivers some help against integer overflows */
  if (new_plane_state->crtc_w > INT_MAX ||
  new_plane_state->crtc_x > INT_MAX - (int32_t) 
new_plane_state->crtc_w ||
@@ -649,49 +705,11 @@ static int drm_atomic_plane_check(const struct 
drm_plane_state *old_plane_state,

  return -ERANGE;
  }
-    fb_width = fb->width << 16;
-    fb_height = fb->height << 16;
-    /* Make sure source coordinates are inside the fb. */
-    if (new_plane_state->src_w > fb_width ||
-    new_plane_state->src_x > fb_width - new_plane_state->src_w ||
-    new_plane_state->src_h > fb_height ||
-    new_plane_state->src_y > fb_height - new_plane_state->

Re: [Freedreno] [PATCH RFC v4 1/7] drm: Introduce solid fill DRM plane property

2023-06-30 Thread Jessica Zhang





On 6/30/2023 3:33 AM, Dmitry Baryshkov wrote:

On 30/06/2023 03:25, Jessica Zhang wrote:

Document and add support for solid_fill property to drm_plane. In
addition, add support for setting and getting the values for solid_fill.

To enable solid fill planes, userspace must assign a property blob to
the "solid_fill" plane property containing the following information:

struct drm_solid_fill_info {
u8 version;
u32 r, g, b;
};

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic_state_helper.c |  9 +
  drivers/gpu/drm/drm_atomic_uapi.c | 55 
+++

  drivers/gpu/drm/drm_blend.c   | 33 +++
  include/drm/drm_blend.h   |  1 +
  include/drm/drm_plane.h   | 43 
  5 files changed, 141 insertions(+)


Also, I think the point which we missed up to now. Could you please add 
both new properties to dri/N/state debugfs?


Hi Dmitry,

Good catch -- acked.

Thanks,

Jessica Zhang



--
With best wishes
Dmitry

Re: [Freedreno] [PATCH v2 2/4] regulator: Introduce Qualcomm REFGEN regulator driver

2023-06-30 Thread Mark Brown

On Thu, Jun 29, 2023 at 10:44:34PM +0200, Konrad Dybcio wrote:
> On 29.06.2023 22:35, Konrad Dybcio wrote:
> > Modern Qualcomm SoCs have a REFGEN (reference voltage generator)
> > regulator, providing reference voltage to on-chip IP, like PHYs.
> > 
> > Add a driver to support toggling that regulator.
> > 
> > Signed-off-by: Konrad Dybcio 
> > ---
> Ugh. Missed the 'const' here and below. LMK if that warrants a resend
> (or.. perhaps you just have other comments)

Please, there was a build error anyway.


signature.asc
Description: PGP signature

Re: [Freedreno] [PATCH v2 1/4] dt-bindings: regulator: Describe Qualcomm REFGEN regulator

2023-06-30 Thread Rob Herring



On Thu, 29 Jun 2023 22:35:41 +0200, Konrad Dybcio wrote:
> Modern Qualcomm SoCs have a REFGEN (reference voltage generator)
> regulator, providing reference voltage to on-chip IP, like PHYs.
> It's controlled through MMIO and we can toggle it or read its state back.
> 
> Describe it.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  .../regulator/qcom,sdm845-refgen-regulator.yaml| 57 
> ++
>  1 file changed, 57 insertions(+)
> 

Reviewed-by: Rob Herring

[Freedreno] [PATCH] drm/msm/a6xx: Fix misleading comment

2023-06-30 Thread Rob Clark

From: Rob Clark 

The range is actually len+1.

Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index eea2e60ce3b7..edf76a4b16bd 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -39,8 +39,8 @@ struct a6xx_gpu {
 
 /*
  * Given a register and a count, return a value to program into
- * REG_CP_PROTECT_REG(n) - this will block both reads and writes for _len
- * registers starting at _reg.
+ * REG_CP_PROTECT_REG(n) - this will block both reads and writes for
+ * _len + 1 registers starting at _reg.
  */
 #define A6XX_PROTECT_NORDWR(_reg, _len) \
((1 << 31) | \
-- 
2.41.0

Re: [Freedreno] [PATCH RFC v4 2/7] drm: Introduce pixel_source DRM plane property

2023-06-30 Thread Sebastian Wick

On Fri, Jun 30, 2023 at 2:26 AM Jessica Zhang  wrote:
>
> Add support for pixel_source property to drm_plane and related
> documentation.
>
> This enum property will allow user to specify a pixel source for the
> plane. Possible pixel sources will be defined in the
> drm_plane_pixel_source enum.
>
> The current possible pixel sources are DRM_PLANE_PIXEL_SOURCE_FB and
> DRM_PLANE_PIXEL_SOURCE_COLOR. The default value is *_SOURCE_FB.
>
> Signed-off-by: Jessica Zhang 
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
>  drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
>  drivers/gpu/drm/drm_blend.c   | 81 
> +++
>  include/drm/drm_blend.h   |  2 +
>  include/drm/drm_plane.h   | 21 
>  5 files changed, 109 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index fe14be2bd2b2..86fb876efbe6 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct 
> drm_plane_state *plane_state,
>
> plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> +   plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
>
> if (plane_state->solid_fill_blob) {
> drm_property_blob_put(plane_state->solid_fill_blob);
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index a28b4ee79444..6e59c21af66b 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -596,6 +596,8 @@ static int drm_atomic_plane_set_property(struct drm_plane 
> *plane,
> drm_property_blob_put(solid_fill);
>
> return ret;
> +   } else if (property == plane->pixel_source_property) {
> +   state->pixel_source = val;
> } else if (property == plane->alpha_property) {
> state->alpha = val;
> } else if (property == plane->blend_mode_property) {
> @@ -671,6 +673,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> } else if (property == plane->solid_fill_property) {
> *val = state->solid_fill_blob ?
> state->solid_fill_blob->base.id : 0;
> +   } else if (property == plane->pixel_source_property) {
> +   *val = state->pixel_source;
> } else if (property == plane->alpha_property) {
> *val = state->alpha;
> } else if (property == plane->blend_mode_property) {
> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> index 38c3c5d6453a..8c100a957ee2 100644
> --- a/drivers/gpu/drm/drm_blend.c
> +++ b/drivers/gpu/drm/drm_blend.c
> @@ -189,6 +189,18 @@
>   * solid_fill is set up with drm_plane_create_solid_fill_property(). It
>   * contains pixel data that drivers can use to fill a plane.
>   *
> + * pixel_source:
> + * pixel_source is set up with drm_plane_create_pixel_source_property().
> + * It is used to toggle the source of pixel data for the plane.
> + *
> + * Possible values:
> + *
> + * "FB":
> + * Framebuffer source
> + *
> + * "COLOR":
> + * solid_fill source
> + *
>   * Note that all the property extensions described here apply either to the
>   * plane or the CRTC (e.g. for the background color, which currently is not
>   * exposed and assumed to be black).
> @@ -648,3 +660,72 @@ int drm_plane_create_solid_fill_property(struct 
> drm_plane *plane)
> return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
> +
> +/**
> + * drm_plane_create_pixel_source_property - create a new pixel source 
> property
> + * @plane: drm plane
> + * @supported_sources: bitmask of supported pixel_sources for the driver (NOT
> + * including DRM_PLANE_PIXEL_SOURCE_FB, as it will be 
> supported
> + * by default).
> + *
> + * This creates a new property describing the current source of pixel data 
> for the
> + * plane.
> + *
> + * The property is exposed to userspace as an enumeration property called
> + * "pixel_source" and has the following enumeration values:
> + *
> + * "FB":
> + * Framebuffer pixel source
> + *
> + * "COLOR":
> + * Solid fill color pixel source

Can we add a "NONE" value?

I know it has been discussed earlier if we *need*  this and I don't
think we do. I just think it would be better API design to disable
planes this way than having to know that a framebuffer pixel source
with a NULL framebuffer disables the plane. Obviously also keep the
old behavior for backwards compatibility.

> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_plane_create_pixel_source_property(struct drm_plane *plane,
> +  unsigned int supported_sources)
> +{
> +

Re: [Freedreno] [PATCH RFC v4 1/7] drm: Introduce solid fill DRM plane property

2023-06-30 Thread Dmitry Baryshkov


On 30/06/2023 03:25, Jessica Zhang wrote:

Document and add support for solid_fill property to drm_plane. In
addition, add support for setting and getting the values for solid_fill.

To enable solid fill planes, userspace must assign a property blob to
the "solid_fill" plane property containing the following information:

struct drm_solid_fill_info {
u8 version;
u32 r, g, b;
};

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic_state_helper.c |  9 +
  drivers/gpu/drm/drm_atomic_uapi.c | 55 +++
  drivers/gpu/drm/drm_blend.c   | 33 +++
  include/drm/drm_blend.h   |  1 +
  include/drm/drm_plane.h   | 43 
  5 files changed, 141 insertions(+)


Also, I think the point which we missed up to now. Could you please add 
both new properties to dri/N/state debugfs?


--
With best wishes
Dmitry

Re: [Freedreno] [PATCH RFC v4 2/7] drm: Introduce pixel_source DRM plane property

2023-06-30 Thread Pekka Paalanen

On Fri, 30 Jun 2023 03:42:28 +0300
Dmitry Baryshkov  wrote:

> On 30/06/2023 03:25, Jessica Zhang wrote:
> > Add support for pixel_source property to drm_plane and related
> > documentation.
> > 
> > This enum property will allow user to specify a pixel source for the
> > plane. Possible pixel sources will be defined in the
> > drm_plane_pixel_source enum.
> > 
> > The current possible pixel sources are DRM_PLANE_PIXEL_SOURCE_FB and
> > DRM_PLANE_PIXEL_SOURCE_COLOR. The default value is *_SOURCE_FB.  
> 
> I think, this should come before the solid fill property addition. First 
> you tell that there is a possibility to define other pixel sources, then 
> additional sources are defined.

Hi,

that would be logical indeed.

> > 
> > Signed-off-by: Jessica Zhang 
> > ---
> >   drivers/gpu/drm/drm_atomic_state_helper.c |  1 +
> >   drivers/gpu/drm/drm_atomic_uapi.c |  4 ++
> >   drivers/gpu/drm/drm_blend.c   | 81 
> > +++
> >   include/drm/drm_blend.h   |  2 +
> >   include/drm/drm_plane.h   | 21 
> >   5 files changed, 109 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
> > b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index fe14be2bd2b2..86fb876efbe6 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -252,6 +252,7 @@ void __drm_atomic_helper_plane_state_reset(struct 
> > drm_plane_state *plane_state,
> >   
> > plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
> > plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
> > +   plane_state->pixel_source = DRM_PLANE_PIXEL_SOURCE_FB;
> >   
> > if (plane_state->solid_fill_blob) {
> > drm_property_blob_put(plane_state->solid_fill_blob);
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> > b/drivers/gpu/drm/drm_atomic_uapi.c
> > index a28b4ee79444..6e59c21af66b 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -596,6 +596,8 @@ static int drm_atomic_plane_set_property(struct 
> > drm_plane *plane,
> > drm_property_blob_put(solid_fill);
> >   
> > return ret;
> > +   } else if (property == plane->pixel_source_property) {
> > +   state->pixel_source = val;
> > } else if (property == plane->alpha_property) {
> > state->alpha = val;
> > } else if (property == plane->blend_mode_property) {  
> 
> I think, it was pointed out in the discussion that drm_mode_setplane() 
> (a pre-atomic IOCTL to turn the plane on and off) should also reset 
> pixel_source to FB.
> 
> > @@ -671,6 +673,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
> > } else if (property == plane->solid_fill_property) {
> > *val = state->solid_fill_blob ?
> > state->solid_fill_blob->base.id : 0;
> > +   } else if (property == plane->pixel_source_property) {
> > +   *val = state->pixel_source;
> > } else if (property == plane->alpha_property) {
> > *val = state->alpha;
> > } else if (property == plane->blend_mode_property) {
> > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
> > index 38c3c5d6453a..8c100a957ee2 100644
> > --- a/drivers/gpu/drm/drm_blend.c
> > +++ b/drivers/gpu/drm/drm_blend.c
> > @@ -189,6 +189,18 @@
> >*solid_fill is set up with 
> > drm_plane_create_solid_fill_property(). It
> >*contains pixel data that drivers can use to fill a plane.
> >*
> > + * pixel_source:
> > + * pixel_source is set up with drm_plane_create_pixel_source_property().
> > + * It is used to toggle the source of pixel data for the plane.

Other sources than the selected one are ignored?

> > + *
> > + * Possible values:

Wouldn't hurt to explicitly mention here that this is an enum.

> > + *
> > + * "FB":
> > + * Framebuffer source
> > + *
> > + * "COLOR":
> > + * solid_fill source

I think these two should be more explicit. Framebuffer source is the
usual source from the property "FB_ID". Solid fill source comes from
the property "solid_fill".

Why "COLOR" and not, say, "SOLID_FILL"?

> > + *
> >* Note that all the property extensions described here apply either to 
> > the
> >* plane or the CRTC (e.g. for the background color, which currently is 
> > not
> >* exposed and assumed to be black).
> > @@ -648,3 +660,72 @@ int drm_plane_create_solid_fill_property(struct 
> > drm_plane *plane)
> > return 0;
> >   }
> >   EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
> > +
> > +/**
> > + * drm_plane_create_pixel_source_property - create a new pixel source 
> > property
> > + * @plane: drm plane
> > + * @supported_sources: bitmask of supported pixel_sources for the driver 
> > (NOT
> > + * including DRM_PLANE_PIXEL_SOURCE_FB, as it will be 
> > supported
> > + * by default).  
> 
> I'd say this is too strong. I'd

Re: [Freedreno] [PATCH RFC v4 1/7] drm: Introduce solid fill DRM plane property

2023-06-30 Thread Pekka Paalanen

On Thu, 29 Jun 2023 17:25:00 -0700
Jessica Zhang  wrote:

> Document and add support for solid_fill property to drm_plane. In
> addition, add support for setting and getting the values for solid_fill.
> 
> To enable solid fill planes, userspace must assign a property blob to
> the "solid_fill" plane property containing the following information:
> 
> struct drm_solid_fill_info {
>   u8 version;
>   u32 r, g, b;
> };
> 
> Signed-off-by: Jessica Zhang 

Hi Jessica,

I've left some general UAPI related comments here.

> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c |  9 +
>  drivers/gpu/drm/drm_atomic_uapi.c | 55 
> +++
>  drivers/gpu/drm/drm_blend.c   | 33 +++
>  include/drm/drm_blend.h   |  1 +
>  include/drm/drm_plane.h   | 43 
>  5 files changed, 141 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 784e63d70a42..fe14be2bd2b2 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -253,6 +253,11 @@ void __drm_atomic_helper_plane_state_reset(struct 
> drm_plane_state *plane_state,
>   plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>   plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>  
> + if (plane_state->solid_fill_blob) {
> + drm_property_blob_put(plane_state->solid_fill_blob);
> + plane_state->solid_fill_blob = NULL;
> + }
> +
>   if (plane->color_encoding_property) {
>   if (!drm_object_property_get_default_value(&plane->base,
>  
> plane->color_encoding_property,
> @@ -335,6 +340,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct 
> drm_plane *plane,
>   if (state->fb)
>   drm_framebuffer_get(state->fb);
>  
> + if (state->solid_fill_blob)
> + drm_property_blob_get(state->solid_fill_blob);
> +
>   state->fence = NULL;
>   state->commit = NULL;
>   state->fb_damage_clips = NULL;
> @@ -384,6 +392,7 @@ void __drm_atomic_helper_plane_destroy_state(struct 
> drm_plane_state *state)
>   drm_crtc_commit_put(state->commit);
>  
>   drm_property_blob_put(state->fb_damage_clips);
> + drm_property_blob_put(state->solid_fill_blob);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index d867e7f9f2cd..a28b4ee79444 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct 
> drm_connector_state *conn_state,
>  }
>  EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
>  
> +static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
> + struct drm_property_blob *blob)
> +{
> + int ret = 0;
> + int blob_version;
> +
> + if (blob == state->solid_fill_blob)
> + return 0;
> +
> + drm_property_blob_put(state->solid_fill_blob);
> + state->solid_fill_blob = NULL;

Is it ok to destroy old state before it is guaranteed that the new
state is accepted?

> +
> + memset(&state->solid_fill, 0, sizeof(state->solid_fill));
> +
> + if (blob) {
> + struct drm_solid_fill_info *user_info = (struct 
> drm_solid_fill_info *)blob->data;
> +
> + if (blob->length != sizeof(struct drm_solid_fill_info)) {
> + drm_dbg_atomic(state->plane->dev,
> +"[PLANE:%d:%s] bad solid fill blob 
> length: %zu\n",
> +state->plane->base.id, 
> state->plane->name,
> +blob->length);
> + return -EINVAL;
> + }
> +
> + blob_version = user_info->version;
> +
> + /* Add more versions if necessary */
> + if (blob_version == 1) {
> + state->solid_fill.r = user_info->r;
> + state->solid_fill.g = user_info->g;
> + state->solid_fill.b = user_info->b;
> + } else {
> + drm_dbg_atomic(state->plane->dev,
> +"[PLANE:%d:%s] failed to set solid fill 
> (ret=%d)\n",
> +state->plane->base.id, 
> state->plane->name,
> +ret);
> + return -EINVAL;

Btw. how does the atomic check machinery work here?

I expect that a TEST_ONLY atomic commit will do all the above checks
and return failure if anything is not right. Right?

> + }
> + state->solid_fill_blob = drm_property_blob_get(blob);
> + }
> +
> + return ret;
> +}
> +
>  static void set_out_fence_for_crtc(struct drm_atomic_state *state,
>

Re: [Freedreno] [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property

2023-06-30 Thread Pekka Paalanen

On Thu, 29 Jun 2023 17:25:06 -0700
Jessica Zhang  wrote:

> Drop DPU_PLANE_COLOR_FILL_FLAG and check the DRM solid_fill property to
> determine if the plane is solid fill. In addition drop the DPU plane
> color_fill field as we can now use drm_plane_state.solid_fill instead,
> and pass in drm_plane_state.alpha to _dpu_plane_color_fill_pipe() to
> allow userspace to configure the alpha value for the solid fill color.
> 
> Signed-off-by: Jessica Zhang 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 21 +++--
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 4476722f03bb..11d4fb771a1f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -42,7 +42,6 @@
>  #define SHARP_SMOOTH_THR_DEFAULT 8
>  #define SHARP_NOISE_THR_DEFAULT  2
>  
> -#define DPU_PLANE_COLOR_FILL_FLAGBIT(31)
>  #define DPU_ZPOS_MAX 255
>  
>  /*
> @@ -82,7 +81,6 @@ struct dpu_plane {
>  
>   enum dpu_sspp pipe;
>  
> - uint32_t color_fill;
>   bool is_error;
>   bool is_rt_pipe;
>   const struct dpu_mdss_cfg *catalog;
> @@ -606,6 +604,17 @@ static void _dpu_plane_color_fill_pipe(struct 
> dpu_plane_state *pstate,
>   _dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, pstate->rotation);
>  }
>  
> +static uint32_t _dpu_plane_get_fill_color(struct drm_solid_fill solid_fill)
> +{
> + uint32_t ret = 0;
> +
> + ret |= ((uint8_t) solid_fill.b) << 16;
> + ret |= ((uint8_t) solid_fill.g) << 8;
> + ret |= ((uint8_t) solid_fill.r);

solid_fill.r, g and b are uint32_t, yes?

What's the value encoding in the UAPI? That doc was missing.

I wouldn't expect the UAPI to use 32-bit variables if it was
essentially 8-bit, so this conversion looks wrong.

Nominal color value 1.0 in u8 is 0xff. The same in u32 is probably
0x? So a simple cast to u8 won't work. You'd want to take the
upper 8 bits instead.


Thanks,
pq

> +
> + return ret;
> +}
> +
>  /**
>   * _dpu_plane_color_fill - enables color fill on plane
>   * @pdpu:   Pointer to DPU plane object
> @@ -977,9 +986,9 @@ void dpu_plane_flush(struct drm_plane *plane)
>   if (pdpu->is_error)
>   /* force white frame with 100% alpha pipe output on error */
>   _dpu_plane_color_fill(pdpu, 0xFF, 0xFF);
> - else if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG)
> - /* force 100% alpha */
> - _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> + else if (drm_plane_solid_fill_enabled(plane->state))
> + _dpu_plane_color_fill(pdpu, 
> _dpu_plane_get_fill_color(plane->state->solid_fill),
> + plane->state->alpha);
>   else {
>   dpu_plane_flush_csc(pdpu, &pstate->pipe);
>   dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
> @@ -1024,7 +1033,7 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane 
> *plane,
>   }
>  
>   /* override for color fill */
> - if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
> + if (drm_plane_solid_fill_enabled(plane->state)) {
>   _dpu_plane_set_qos_ctrl(plane, pipe, false);
>  
>   /* skip remaining processing on color fill */
> 



pgpXih8YBTDXZ.pgp
Description: OpenPGP digital signature

Re: [Freedreno] [PATCH RFC v4 6/7] drm/msm/dpu: Allow NULL FBs in atomic commit

2023-06-30 Thread Pekka Paalanen

On Fri, 30 Jun 2023 03:52:37 +0300
Dmitry Baryshkov  wrote:

> On 30/06/2023 03:25, Jessica Zhang wrote:
> > Since solid fill planes allow for a NULL framebuffer in a valid commit,
> > add NULL framebuffer checks to atomic commit calls within DPU.
> > 
> > Signed-off-by: Jessica Zhang 
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  9 ++-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 45 
> > +++
> >   2 files changed, 36 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 1edf2b6b0a26..d1b37d2cc202 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -451,6 +451,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
> > *crtc,
> > struct drm_plane_state *state;
> > struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> > struct dpu_plane_state *pstate = NULL;
> > +   const struct msm_format *fmt;
> > struct dpu_format *format;
> > struct dpu_hw_ctl *ctl = mixer->lm_ctl;
> >   
> > @@ -470,7 +471,13 @@ static void _dpu_crtc_blend_setup_mixer(struct 
> > drm_crtc *crtc,
> > pstate = to_dpu_plane_state(state);
> > fb = state->fb;
> >   
> > -   format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
> > +   if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb)
> > +   fmt = msm_framebuffer_format(pstate->base.fb);
> > +   else
> > +   fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
> > +   DRM_FORMAT_RGBA, 0);  
> 
> The DRM_FORMAT_RGBA should be defined somewhere in patch 1 as format 
> for the solid_fill, then that define can be used in this patch.

Isn't this just a driver-specific decision to convert a RGB323232
solid_fill to be presented as a RGBA?

Though, below there is ABGR used for something... inconsistent?


Thanks,
pq

> 
> > +
> > +   format = to_dpu_format(fmt);
> >   
> > if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
> > bg_alpha_enable = true;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 5f0984ce62b1..4476722f03bb 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -837,8 +837,10 @@ static int dpu_plane_atomic_check(struct drm_plane 
> > *plane,
> >   
> > pipe_cfg->dst_rect = new_plane_state->dst;
> >   
> > -   fb_rect.x2 = new_plane_state->fb->width;
> > -   fb_rect.y2 = new_plane_state->fb->height;
> > +   if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && 
> > new_plane_state->fb) {
> > +   fb_rect.x2 = new_plane_state->fb->width;
> > +   fb_rect.y2 = new_plane_state->fb->height;
> > +   }
> >   
> > /* Ensure fb size is supported */
> > if (drm_rect_width(&fb_rect) > MAX_IMG_WIDTH ||
> > @@ -848,10 +850,13 @@ 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;
> >   
> > +   if (drm_plane_solid_fill_enabled(new_plane_state))
> > +   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
> > +   else
> > +   fmt = 
> > to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
> > +
> > if (drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) {
> > /*
> >  * In parallel multirect case only the half of the usual width
> > @@ -1082,21 +1087,32 @@ static void dpu_plane_sspp_atomic_update(struct 
> > drm_plane *plane)
> > struct drm_crtc *crtc = state->crtc;
> > struct drm_framebuffer *fb = state->fb;
> > bool is_rt_pipe;
> > -   const struct dpu_format *fmt =
> > -   to_dpu_format(msm_framebuffer_format(fb));
> > +   const struct dpu_format *fmt;
> > struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> > struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> > struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> > struct msm_gem_address_space *aspace = kms->base.aspace;
> > struct dpu_hw_fmt_layout layout;
> > bool layout_valid = false;
> > -   int ret;
> >   
> > -   ret = dpu_format_populate_layout(aspace, fb, &layout);
> > -   if (ret)
> > -   DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
> > -   else
> > -   layout_valid = true;
> > +   if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) {
> > +   int ret;
> > +
> > +   fmt = to_dpu_format(msm_framebuffer_format(fb));
> > +
> > +   ret = dpu_format_populate_layout(aspace, fb, &layout);
> > +   if (ret)
> > +   DPU_ERROR_PLANE(pdpu, "fai

Re: [Freedreno] [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property

Re: [Freedreno] [PATCH RFC v4 6/7] drm/msm/dpu: Allow NULL FBs in atomic commit

Re: [Freedreno] [PATCH v2] drm/msm/adreno: Assign revn to A635

Re: [Freedreno] [PATCH RFC v4 5/7] drm/msm/dpu: Add solid fill and pixel source properties

Re: [Freedreno] [PATCH RFC v4 4/7] drm/atomic: Loosen FB atomic checks

Re: [Freedreno] [PATCH 0/6] Add support to print sub block registers in dpu hw catalog

[Freedreno] [PATCH v2] drm/msm/adreno: Assign revn to A635

Re: [Freedreno] [PATCH RFC v4 2/7] drm: Introduce pixel_source DRM plane property

[Freedreno] [PATCH 09/15] drm/msm/hdmi: add runtime PM calls to DDC transfer function

[Freedreno] [PATCH 14/15] drm/msm/hdmi: ensure that HDMI is one if HPD is requested

[Freedreno] [PATCH 15/15] drm/msm/hdmi: wire in hpd_enable/hpd_disable bridge ops

[Freedreno] [PATCH 12/15] drm/msm/hdmi: expand the HDMI_CFG macro

[Freedreno] [PATCH 04/15] drm/msm/hdmi: switch to atomic_pre_enable/post_disable

[Freedreno] [PATCH 11/15] drm/msm/hdmi: rename hpd_clks to pwr_clks

[Freedreno] [PATCH 05/15] drm/msm/hdmi: set infoframes on all pre_enable calls

[Freedreno] [PATCH 07/15] drm/msm/hdmi: switch to clk_bulk API

[Freedreno] [PATCH 13/15] drm/msm/hdmi: drop hpd-gpios support

[Freedreno] [PATCH 10/15] drm/msm/hdmi: implement proper runtime PM handling

[Freedreno] [PATCH 06/15] drm/msm/hdmi: drop clock frequency assignment

[Freedreno] [PATCH 08/15] drm/msm/hdmi: switch to pm_runtime_resume_and_get()

[Freedreno] [PATCH 02/15] drm/msm/hdmi: simplify extp clock handling

[Freedreno] [PATCH 03/15] drm/msm/hdmi: correct indentation of HDMI bridge functions

[Freedreno] [PATCH 01/15] drm/msm/hdmi: move the alt_iface clock to the hpd list

[Freedreno] [PATCH 00/15] drm/msm/hdmi: rework and fix the HPD even generation

Re: [Freedreno] [PATCH] drm/msm/adreno: Assign revn to A635

Re: [Freedreno] [PATCH RFC v4 3/7] drm/atomic: Move framebuffer checks to helper

Re: [Freedreno] [PATCH RFC v4 1/7] drm: Introduce solid fill DRM plane property

Re: [Freedreno] [PATCH v2 2/4] regulator: Introduce Qualcomm REFGEN regulator driver

Re: [Freedreno] [PATCH v2 1/4] dt-bindings: regulator: Describe Qualcomm REFGEN regulator

[Freedreno] [PATCH] drm/msm/a6xx: Fix misleading comment

Re: [Freedreno] [PATCH RFC v4 2/7] drm: Introduce pixel_source DRM plane property

Re: [Freedreno] [PATCH RFC v4 1/7] drm: Introduce solid fill DRM plane property

Re: [Freedreno] [PATCH RFC v4 2/7] drm: Introduce pixel_source DRM plane property

Re: [Freedreno] [PATCH RFC v4 1/7] drm: Introduce solid fill DRM plane property

Re: [Freedreno] [PATCH RFC v4 7/7] drm/msm/dpu: Use DRM solid_fill property

Re: [Freedreno] [PATCH RFC v4 6/7] drm/msm/dpu: Allow NULL FBs in atomic commit

36 matches

Site Navigation

Mail list logo

Footer information