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

2022-10-31 Thread Jessica Zhang




On 10/29/2022 4:40 AM, Dmitry Baryshkov wrote:

On 29/10/2022 01:59, 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.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  7 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 ++-
  2 files changed, 48 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..157698b4f234 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -441,7 +441,12 @@ 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)
+    format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));

+    else if (state->color_fill && !state->color_fill_format)
+    format = dpu_get_dpu_format(DRM_FORMAT_ABGR);


As I wrote in the review of the earlier patch, this disallows using 
black as the plane fill colour. Not to mention that using ABGR should be 
explicit rather than implicit.


Hey Dmitry,

Acked.

Thanks,

Jessica Zhang




+    else
+    format = dpu_get_dpu_format(state->color_fill_format);
  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 658005f609f4..f3be37e97b64 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -103,7 +103,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;
@@ -697,7 +696,10 @@ static int _dpu_plane_color_fill(struct dpu_plane 
*pdpu,

   * select fill format to match user property expectation,
   * h/w only supports RGB variants
   */
-    fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+    if (plane->state->color_fill && !plane->state->color_fill_format)
+    fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+    else
+    fmt = dpu_get_dpu_format(plane->state->color_fill_format);
  /* update sspp */
  if (fmt && pdpu->pipe_hw->ops.setup_solidfill) {
@@ -720,6 +722,10 @@ static int _dpu_plane_color_fill(struct dpu_plane 
*pdpu,

  fmt, DPU_SSPP_SOLID_FILL,
  pstate->multirect_index);
+    /* skip remaining processing on color fill */
+    if (!plane->state->fb)
+    return 0;
+
  if (pdpu->pipe_hw->ops.setup_rects)
  pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw,
  _cfg,
@@ -999,12 +1005,21 @@ 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 if (new_plane_state->color_fill) {
+    if (new_plane_state->color_fill_format)
+    fmt = 
dpu_get_dpu_format(new_plane_state->color_fill_format);

+    else
+    fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+    }
  min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -1016,7 +1031,7 @@ static int dpu_plane_atomic_check(struct 
drm_plane *plane,

  return -EINVAL;
  /* check src bounds */
-    } else if (!dpu_plane_validate_src(, _rect, min_src_size)) {
+    } else if (new_plane_state->fb && !dpu_plane_validate_src(, 
_rect, min_src_size)) {

  DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
  DRM_RECT_ARG());
  return -E2BIG;
@@ -1084,9 +1099,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)
+    else if (!(plane->state->fb) && plane->state->color_fill)
  /* force 100% alpha */
-    _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
+    _dpu_plane_color_fill(pdpu, plane->state->color_fill, 0xFF);
  else if (pdpu->pipe_hw && pdpu->pipe_hw->ops.setup_csc) {
  const struct dpu_format *fmt = 

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

2022-10-29 Thread Dmitry Baryshkov

On 29/10/2022 01:59, 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.

Signed-off-by: Jessica Zhang 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  7 ++-
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 ++-
  2 files changed, 48 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..157698b4f234 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -441,7 +441,12 @@ 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)
+   format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+   else if (state->color_fill && !state->color_fill_format)
+   format = dpu_get_dpu_format(DRM_FORMAT_ABGR);


As I wrote in the review of the earlier patch, this disallows using 
black as the plane fill colour. Not to mention that using ABGR should be 
explicit rather than implicit.



+   else
+   format = dpu_get_dpu_format(state->color_fill_format);
  
  		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 658005f609f4..f3be37e97b64 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -103,7 +103,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;
@@ -697,7 +696,10 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
 * select fill format to match user property expectation,
 * h/w only supports RGB variants
 */
-   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   if (plane->state->color_fill && !plane->state->color_fill_format)
+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   else
+   fmt = dpu_get_dpu_format(plane->state->color_fill_format);
  
  	/* update sspp */

if (fmt && pdpu->pipe_hw->ops.setup_solidfill) {
@@ -720,6 +722,10 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
fmt, DPU_SSPP_SOLID_FILL,
pstate->multirect_index);
  
+		/* skip remaining processing on color fill */

+   if (!plane->state->fb)
+   return 0;
+
if (pdpu->pipe_hw->ops.setup_rects)
pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw,
_cfg,
@@ -999,12 +1005,21 @@ 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 if (new_plane_state->color_fill) {
+   if (new_plane_state->color_fill_format)
+   fmt = 
dpu_get_dpu_format(new_plane_state->color_fill_format);
+   else
+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   }
  
  	min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
  
@@ -1016,7 +1031,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,

return -EINVAL;
  
  	/* check src bounds */

-   } else if (!dpu_plane_validate_src(, _rect, min_src_size)) {
+   } else if (new_plane_state->fb && !dpu_plane_validate_src(, 
_rect, min_src_size)) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG());
return -E2BIG;
@@ -1084,9 +1099,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)
+   else if (!(plane->state->fb) && plane->state->color_fill)
/* force 100% alpha 

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

2022-10-28 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.

Signed-off-by: Jessica Zhang 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  7 ++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 66 ++-
 2 files changed, 48 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..157698b4f234 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -441,7 +441,12 @@ 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)
+   format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));
+   else if (state->color_fill && !state->color_fill_format)
+   format = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   else
+   format = dpu_get_dpu_format(state->color_fill_format);
 
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 658005f609f4..f3be37e97b64 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -103,7 +103,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;
@@ -697,7 +696,10 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
 * select fill format to match user property expectation,
 * h/w only supports RGB variants
 */
-   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   if (plane->state->color_fill && !plane->state->color_fill_format)
+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   else
+   fmt = dpu_get_dpu_format(plane->state->color_fill_format);
 
/* update sspp */
if (fmt && pdpu->pipe_hw->ops.setup_solidfill) {
@@ -720,6 +722,10 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
fmt, DPU_SSPP_SOLID_FILL,
pstate->multirect_index);
 
+   /* skip remaining processing on color fill */
+   if (!plane->state->fb)
+   return 0;
+
if (pdpu->pipe_hw->ops.setup_rects)
pdpu->pipe_hw->ops.setup_rects(pdpu->pipe_hw,
_cfg,
@@ -999,12 +1005,21 @@ 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 if (new_plane_state->color_fill) {
+   if (new_plane_state->color_fill_format)
+   fmt = 
dpu_get_dpu_format(new_plane_state->color_fill_format);
+   else
+   fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR);
+   }
 
min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
 
@@ -1016,7 +1031,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
 
/* check src bounds */
-   } else if (!dpu_plane_validate_src(, _rect, min_src_size)) {
+   } else if (new_plane_state->fb && !dpu_plane_validate_src(, 
_rect, min_src_size)) {
DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG());
return -E2BIG;
@@ -1084,9 +1099,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)
+   else if (!(plane->state->fb) && plane->state->color_fill)
/* force 100% alpha */
-   _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
+   _dpu_plane_color_fill(pdpu, plane->state->color_fill, 0xFF);
else if