[RFC PATCH v3 0/3] Support for Solid Fill Planes

2023-01-04 Thread Jessica Zhang
Introduce and add support for a solid_fill property. When the solid_fill
property is set, and the framebuffer is set to NULL, memory fetch will be
disabled.

In addition, loosen the NULL FB checks within the atomic commit callstack
to allow a NULL FB when the solid_fill property is set and add FB checks
in methods where the FB was previously assumed to be non-NULL.

Finally, have the DPU driver use drm_plane_state.solid_fill and instead of
dpu_plane_state.color_fill, and add extra checks in the DPU atomic commit
callstack to account for a NULL FB in cases where solid_fill is set.

Some drivers support hardware that have optimizations for solid fill
planes. This series aims to expose these capabilities to userspace as
some compositors have a solid fill flag (ex. SOLID_COLOR in the Android
hardware composer HAL) that can be set by apps like the Android Gears
app.

Userspace can set the solid_fill property to a blob containing the
appropriate version number and solid fill color (in RGB323232 format) and
setting the framebuffer to NULL.

Note: Currently, there's only one version of the solid_fill blob property.
However if other drivers want to support a similar feature, but require
more than just the solid fill color, they can extend this feature by
creating additional versions of the drm_solid_fill struct.

Changes in V2:
- Dropped SOLID_FILL_FORMAT property (Simon)
- Switched to implementing solid_fill property as a blob (Simon, Dmitry)
- Changed to checks for if solid_fill_blob is set (Dmitry)
- Abstracted (plane_state && !solid_fill_blob) checks to helper method
  (Dmitry)
- Removed DPU_PLANE_COLOR_FILL_FLAG
- Fixed whitespace and indentation issues (Dmitry)

Changes in V3:
- Fixed some logic errors in atomic checks (Dmitry)
- Introduced drm_plane_has_visible_data() and drm_atomic_check_fb() helper
  methods (Dmitry)

Jessica Zhang (3):
  drm: Introduce solid fill property for drm plane
  drm: Adjust atomic checks for solid fill color
  drm/msm/dpu: Use color_fill property for DPU planes

 drivers/gpu/drm/drm_atomic.c  | 136 +-
 drivers/gpu/drm/drm_atomic_helper.c   |  34 +++---
 drivers/gpu/drm/drm_atomic_state_helper.c |   9 ++
 drivers/gpu/drm/drm_atomic_uapi.c |  59 ++
 drivers/gpu/drm/drm_blend.c   |  17 +++
 drivers/gpu/drm/drm_plane.c   |   8 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |   9 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  65 +++
 include/drm/drm_atomic_helper.h   |   5 +-
 include/drm/drm_blend.h   |   1 +
 include/drm/drm_plane.h   |  62 ++
 11 files changed, 302 insertions(+), 103 deletions(-)

-- 
2.38.1



[RFC PATCH v3 2/3] drm: Adjust atomic checks for solid fill color

2023-01-04 Thread Jessica Zhang
Loosen the requirements for atomic and legacy commit so that, in cases
where solid fill planes is enabled (and FB_ID is NULL), the commit can
still go through.

In addition, add framebuffer NULL checks in other areas to account for
FB being NULL when solid fill is enabled.

Changes in V2:
- Changed to checks for if solid_fill_blob is set (Dmitry)
- Abstracted (plane_state && !solid_fill_blob) checks to helper method
  (Dmitry)
- Fixed indentation issue (Dmitry)

Changes in V3:
- Created drm_plane_has_visible_data() helper and corrected CRTC and FB
  NULL-check logic (Dmitry)
- Merged `if (fb)` blocks in drm_atomic_plane_check() and abstracted
  them into helper method (Dmitry)
- Inverted `if (solid_fill_enabled) else if (fb)` check order (Dmitry)
- Fixed indentation (Dmitry)

Signed-off-by: Jessica Zhang 
---
 drivers/gpu/drm/drm_atomic.c| 136 
 drivers/gpu/drm/drm_atomic_helper.c |  34 ---
 drivers/gpu/drm/drm_plane.c |   8 +-
 include/drm/drm_atomic_helper.h |   5 +-
 include/drm/drm_plane.h |  19 
 5 files changed, 124 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f197f59f6d99..63f34b430479 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,13 +666,12 @@ 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 */
-   if (crtc && !fb) {
+   /* When solid_fill is disabled,
+* eithe

[RFC PATCH v3 1/3] drm: Introduce solid fill property for drm plane

2023-01-04 Thread Jessica Zhang
Add support for solid_fill property to drm_plane. In addition, add
support for setting and getting the values for solid_fill.

solid_fill holds data for supporting solid fill planes. The property
accepts an RGB323232 value and the driver data is formatted as such:

struct drm_solid_fill {
u32 r;
u32 g;
u32 b;
};

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

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

Changes in V2:
- Changed solid_fill property to a property blob (Simon, Dmitry)
- Added drm_solid_fill struct (Simon)
- Added drm_solid_fill_info struct (Simon)

Changes in V3:
- Corrected typo in drm_solid_fill struct documentation

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

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index dfb57217253b..c96fd1f2ad99 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 c06d0639d552..8a1d2fb7a757 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -316,6 +316,55 @@ drm_atomic_set_crtc_for_connector(struct 
drm_connector_state *conn_state,
 }
 EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
 
+static void drm_atomic_convert_solid_fill_info(struct drm_solid_fill *out,
+   struct drm_solid_fill_info *in)
+{
+   out->r = in->r;
+   out->g = in->g;
+   out->b = in->b;
+}
+
+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;
+
+   memset(&state->solid_fill, 0, sizeof(state->solid_fill));
+
+   if (blob) {
+   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 = ((struct drm_solid_fill_info 
*)blob->data)->version;
+
+   /* Append with more versions if necessary */
+   if (blob_version == 1) {
+   drm_atomic_convert_solid_fill_info(&state->solid_fill, 
blob->data);
+   } 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;
+   }
+   state->solid_fill_blob = drm_property_blob_get(blob);
+   }
+
+   return ret;
+}
+
 static void set_out_fence_for_crtc(struct drm_atomic_state *state,
   struct drm_crtc *crtc, s32 __user *fence_ptr)
 {
@@ -544,

[RFC PATCH v3 3/3] drm/msm/dpu: Use color_fill property for DPU planes

2023-01-04 Thread Jessica Zhang
Initialize and use the color_fill properties for planes in DPU driver. In
addition, relax framebuffer requirements within atomic commit path and
add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG
as it's unused.

Changes since V2:
- Fixed dropped 'const' warning
- Dropped use of solid_fill_format
- Switched to using drm_plane_solid_fill_enabled helper method
- Added helper to convert color fill to BGR888 (Rob)
- Added support for solid fill on planes of varying sizes
- Removed DPU_PLANE_COLOR_FILL_FLAG

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

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..0695b70ea1b7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -409,6 +409,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;
 
@@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
sspp_idx - SSPP_VIG0,
state->fb ? state->fb->base.id : -1);
 
-   format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+   if (pstate->base.fb)
+   fmt = msm_framebuffer_format(pstate->base.fb);
+   else
+   fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
+   DRM_FORMAT_ABGR, 0);
+
+   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 86719020afe2..51a7507373f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -44,7 +44,6 @@
 
 #define DPU_NAME_SIZE  12
 
-#define DPU_PLANE_COLOR_FILL_FLAG  BIT(31)
 #define DPU_ZPOS_MAX 255
 
 /* multirect rect index */
@@ -105,7 +104,6 @@ struct dpu_plane {
enum dpu_sspp pipe;
 
struct dpu_hw_pipe *pipe_hw;
-   uint32_t color_fill;
bool is_error;
bool is_rt_pipe;
const struct dpu_mdss_cfg *catalog;
@@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu,
&scaler3_cfg);
 }
 
+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);
+
+   return ret;
+}
+
 /**
  * _dpu_plane_color_fill - enables color fill on plane
  * @pdpu:   Pointer to DPU plane object
@@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
 
dst = drm_plane_state_dest(new_plane_state);
 
-   fb_rect.x2 = new_plane_state->fb->width;
-   fb_rect.y2 = new_plane_state->fb->height;
+   if (new_plane_state->fb) {
+   fb_rect.x2 = new_plane_state->fb->width;
+   fb_rect.y2 = new_plane_state->fb->height;
+   }
 
max_linewidth = pdpu->catalog->caps->max_linewidth;
 
-   fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+   if (new_plane_state->fb)
+   fmt = 
to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+   else
+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
 
min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
 
@@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
 
/* check src bounds */
-   } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) {
+   } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, 
&fb_rect, min_src_size)) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG(&src));
return -E2BIG;
@@ -1086,9 +1100,10 @@ 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)
+   else if (!(plane->state->fb) && 
drm_plane_solid_fill_enabled(plane->state))
/* force 100% alpha */
-   _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
+   _dpu_plane_color_fil

Re: [RFC PATCH v3 1/3] drm: Introduce solid fill property for drm plane

2023-01-04 Thread Dmitry Baryshkov

On 05/01/2023 01:40, Jessica Zhang wrote:

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

solid_fill holds data for supporting solid fill planes. The property
accepts an RGB323232 value and the driver data is formatted as such:

struct drm_solid_fill {
u32 r;
u32 g;
u32 b;
};


This description is unnecessary (and confusing), since you describe 
drm_solid_fill_info below.




To enable solid fill planes, userspace must assigned solid_fill to a


must assign. Also the phrasing seems strange. The blob is assigned to 
the 'solid_fill' property, not the other way around.



property blob containing the following information:


This should clearly describe if solid_fill overrides FB or if FB 
overrides solid_fill. Also please extend properties documentation in 
Documentation/gpu/drm-kms.rst.




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

Changes in V2:
- Changed solid_fill property to a property blob (Simon, Dmitry)
- Added drm_solid_fill struct (Simon)
- Added drm_solid_fill_info struct (Simon)

Changes in V3:
- Corrected typo in drm_solid_fill struct documentation

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

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c 
b/drivers/gpu/drm/drm_atomic_state_helper.c
index dfb57217253b..c96fd1f2ad99 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 c06d0639d552..8a1d2fb7a757 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -316,6 +316,55 @@ drm_atomic_set_crtc_for_connector(struct 
drm_connector_state *conn_state,
  }
  EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
  
+static void drm_atomic_convert_solid_fill_info(struct drm_solid_fill *out,

+   struct drm_solid_fill_info *in)


No need for a separate function, you can inline this.


+{
+   out->r = in->r;
+   out->g = in->g;
+   out->b = in->b;
+}
+
+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;
+
+   memset(&state->solid_fill, 0, sizeof(state->solid_fill));
+
+   if (blob) {
+   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 = ((struct drm_solid_fill_info 
*)blob->data)->version;


Please assign blob->data to temporary var.


+
+   /* Append with more versions if necessary */


s/Append/Add/


+   if (blob_version == 1) {
+   drm_atomic_convert_solid_fill_info(&state->solid_fill, 
blob->data);
+   } else {
+   drm_dbg_atom

Re: [RFC PATCH v3 2/3] drm: Adjust atomic checks for solid fill color

2023-01-04 Thread Dmitry Baryshkov

On 05/01/2023 01:40, Jessica Zhang wrote:

Loosen the requirements for atomic and legacy commit so that, in cases
where solid fill planes is enabled (and FB_ID is NULL), the commit can
still go through.

In addition, add framebuffer NULL checks in other areas to account for
FB being NULL when solid fill is enabled.

Changes in V2:
- Changed to checks for if solid_fill_blob is set (Dmitry)
- Abstracted (plane_state && !solid_fill_blob) checks to helper method
   (Dmitry)
- Fixed indentation issue (Dmitry)

Changes in V3:
- Created drm_plane_has_visible_data() helper and corrected CRTC and FB
   NULL-check logic (Dmitry)
- Merged `if (fb)` blocks in drm_atomic_plane_check() and abstracted
   them into helper method (Dmitry)
- Inverted `if (solid_fill_enabled) else if (fb)` check order (Dmitry)
- Fixed indentation (Dmitry)

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/drm_atomic.c| 136 
  drivers/gpu/drm/drm_atomic_helper.c |  34 ---
  drivers/gpu/drm/drm_plane.c |   8 +-
  include/drm/drm_atomic_helper.h |   5 +-
  include/drm/drm_plane.h |  19 
  5 files changed, 124 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f197f59f6d99..63f34b430479 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)


This change should go to a separate patch. Please don't mix refactoring 
(moving of the code) with the actual functionality changes.



+{
+   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,13 +666,12 @@ 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

Re: [RFC PATCH v3 1/3] drm: Introduce solid fill property for drm plane

2023-01-04 Thread Dmitry Baryshkov

On 05/01/2023 01:40, Jessica Zhang wrote:

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

solid_fill holds data for supporting solid fill planes. The property
accepts an RGB323232 value and the driver data is formatted as such:

struct drm_solid_fill {
u32 r;
u32 g;
u32 b;
};

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

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


BTW: should we add support for alpha too? DPU hardware supports using 
RGBA as a fill colour format, doesn't it?


But then we face the obvious question, how do we communicate to 
userspace if the hardware support RGB or RGBA?



};

Changes in V2:
- Changed solid_fill property to a property blob (Simon, Dmitry)
- Added drm_solid_fill struct (Simon)
- Added drm_solid_fill_info struct (Simon)

Changes in V3:
- Corrected typo in drm_solid_fill struct documentation

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


--
With best wishes
Dmitry



Re: [RFC PATCH v3 3/3] drm/msm/dpu: Use color_fill property for DPU planes

2023-01-04 Thread Dmitry Baryshkov

On 05/01/2023 01:40, Jessica Zhang wrote:

Initialize and use the color_fill properties for planes in DPU driver. In
addition, relax framebuffer requirements within atomic commit path and
add checks for NULL framebuffers. Finally, drop DPU_PLANE_COLOR_FILL_FLAG
as it's unused.

Changes since V2:
- Fixed dropped 'const' warning
- Dropped use of solid_fill_format
- Switched to using drm_plane_solid_fill_enabled helper method
- Added helper to convert color fill to BGR888 (Rob)
- Added support for solid fill on planes of varying sizes
- Removed DPU_PLANE_COLOR_FILL_FLAG

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

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..0695b70ea1b7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -409,6 +409,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;
  
@@ -441,7 +442,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,

sspp_idx - SSPP_VIG0,
state->fb ? state->fb->base.id : -1);
  
-		format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));

+   if (pstate->base.fb)
+   fmt = msm_framebuffer_format(pstate->base.fb);
+   else
+   fmt = dpu_get_msm_format(&_dpu_crtc_get_kms(crtc)->base,
+   DRM_FORMAT_ABGR, 0);
+
+   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 86719020afe2..51a7507373f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -44,7 +44,6 @@
  
  #define DPU_NAME_SIZE  12
  
-#define DPU_PLANE_COLOR_FILL_FLAG	BIT(31)

  #define DPU_ZPOS_MAX 255
  
  /* multirect rect index */

@@ -105,7 +104,6 @@ struct dpu_plane {
enum dpu_sspp pipe;
  
  	struct dpu_hw_pipe *pipe_hw;

-   uint32_t color_fill;
bool is_error;
bool is_rt_pipe;
const struct dpu_mdss_cfg *catalog;
@@ -678,6 +676,17 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu,
&scaler3_cfg);
  }
  
+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);
+
+   return ret;
+}
+
  /**
   * _dpu_plane_color_fill - enables color fill on plane
   * @pdpu:   Pointer to DPU plane object
@@ -1001,12 +1010,17 @@ static int dpu_plane_atomic_check(struct drm_plane 
*plane,
  
  	dst = drm_plane_state_dest(new_plane_state);
  
-	fb_rect.x2 = new_plane_state->fb->width;

-   fb_rect.y2 = new_plane_state->fb->height;
+   if (new_plane_state->fb) {
+   fb_rect.x2 = new_plane_state->fb->width;
+   fb_rect.y2 = new_plane_state->fb->height;
+   }
  
  	max_linewidth = pdpu->catalog->caps->max_linewidth;
  
-	fmt = to_dpu_format(msm_framebuffer_format(new_plane_state->fb));

+   if (new_plane_state->fb)
+   fmt = 
to_dpu_format(msm_framebuffer_format(new_plane_state->fb));
+   else
+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);


I think this should be more explicit:

if (solid_fill)
   fmt = dpu_get_dpu_format(...)
else
   fmt = to_dpu_format(msm_framebuffer_format(...).

And in the _dpu_crtc_blend_setup_mixer() too.

Maybe the code can be extracted to a helper.

  
  	min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
  
@@ -1018,7 +1032,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,

return -EINVAL;
  
  	/* check src bounds */

-   } else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) {
+   } else if (new_plane_state->fb && !dpu_plane_validate_src(&src, 
&fb_rect, min_src_size)) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG(&src));
return -E2BIG;
@@ -1086,9 +1100,10 @@ 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