diff mbox series

[v3,25/27] drm/msm/dpu: rework static color fill code

Message ID 20230203182132.1307834-26-dmitry.baryshkov@linaro.org
State Superseded
Headers show
Series [v3,01/27] drm/msm/dpu: rename struct dpu_hw_pipe(_cfg) to dpu_hw_sspp(_cfg) | expand

Commit Message

Dmitry Baryshkov Feb. 3, 2023, 6:21 p.m. UTC
Rework static color fill code to separate the pipe / pipe_cfg handling.
This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +++++++++++++----------
 1 file changed, 41 insertions(+), 29 deletions(-)

Comments

Abhinav Kumar Feb. 8, 2023, 10:34 p.m. UTC | #1
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
> Rework static color fill code to separate the pipe / pipe_cfg handling.
> This is a preparation for the r_pipe support.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +++++++++++++----------
>   1 file changed, 41 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 05047192cb37..e2e85688ed3c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -639,20 +639,54 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
>   				fmt);
>   }
>   
> +static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
> +		struct dpu_sw_pipe *pipe,
> +		struct dpu_hw_sspp_cfg *old_pipe_cfg,

Why is this called old_pipe_cfg instead of just pipe_cfg?


> +		u32 fill_color,
> +		const struct dpu_format *fmt)
> +{
> +	struct dpu_hw_sspp_cfg pipe_cfg;
> +
> +	/* update sspp */
> +	if (!pipe->sspp->ops.setup_solidfill)
> +		return 0;

You can just return from here and make this function void?

> +
> +	pipe->sspp->ops.setup_solidfill(pipe, fill_color);
> +
> +	/* override scaler/decimation if solid fill */
> +	pipe_cfg.dst_rect = old_pipe_cfg->dst_rect;
> +
> +	pipe_cfg.src_rect.x1 = 0;
> +	pipe_cfg.src_rect.y1 = 0;
> +	pipe_cfg.src_rect.x2 =
> +		drm_rect_width(&pipe_cfg.dst_rect);
> +	pipe_cfg.src_rect.y2 =
> +		drm_rect_height(&pipe_cfg.dst_rect);
> +
> +	if (pipe->sspp->ops.setup_format)
> +		pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL);
> +
> +	if (pipe->sspp->ops.setup_rects)
> +		pipe->sspp->ops.setup_rects(pipe, &pipe_cfg);
> +
> +	_dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, pstate->rotation);
> +
> +	return 0;
> +}
> +
>   /**
>    * _dpu_plane_color_fill - enables color fill on plane
>    * @pdpu:   Pointer to DPU plane object
>    * @color:  RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red
>    * @alpha:  8-bit fill alpha value, 255 selects 100% alpha
> - * Returns: 0 on success
>    */
> -static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
> +static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
>   		uint32_t color, uint32_t alpha)
>   {
>   	const struct dpu_format *fmt;
>   	const struct drm_plane *plane = &pdpu->base;
>   	struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
> -	struct dpu_hw_sspp_cfg pipe_cfg;
> +	u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24);
>   
>   	DPU_DEBUG_PLANE(pdpu, "\n");
>   
> @@ -661,34 +695,12 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
>   	 * h/w only supports RGB variants
>   	 */
>   	fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888);
> +	/* should not happen ever */
> +	if (!fmt)
> +		return;
>   
>   	/* update sspp */
> -	if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
> -		pstate->pipe.sspp->ops.setup_solidfill(&pstate->pipe,
> -				(color & 0xFFFFFF) | ((alpha & 0xFF) << 24));
> -
> -		/* override scaler/decimation if solid fill */
> -		pipe_cfg.dst_rect = pstate->base.dst;
> -
> -		pipe_cfg.src_rect.x1 = 0;
> -		pipe_cfg.src_rect.y1 = 0;
> -		pipe_cfg.src_rect.x2 =
> -			drm_rect_width(&pipe_cfg.dst_rect);
> -		pipe_cfg.src_rect.y2 =
> -			drm_rect_height(&pipe_cfg.dst_rect);
> -
> -		if (pstate->pipe.sspp->ops.setup_format)
> -			pstate->pipe.sspp->ops.setup_format(&pstate->pipe,
> -					fmt, DPU_SSPP_SOLID_FILL);
> -
> -		if (pstate->pipe.sspp->ops.setup_rects)
> -			pstate->pipe.sspp->ops.setup_rects(&pstate->pipe,
> -					&pipe_cfg);
> -
> -		_dpu_plane_setup_scaler(&pstate->pipe, fmt, true, &pipe_cfg, pstate->rotation);
> -	}
> -
> -	return 0;
> +	_dpu_plane_color_fill_pipe(pstate, &pstate->pipe, &pstate->pipe_cfg, fill_color, fmt);
>   }
>   
>   int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
Dmitry Baryshkov Feb. 9, 2023, 12:53 a.m. UTC | #2
On 09/02/2023 00:34, Abhinav Kumar wrote:
> 
> 
> On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
>> Rework static color fill code to separate the pipe / pipe_cfg handling.
>> This is a preparation for the r_pipe support.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 70 +++++++++++++----------
>>   1 file changed, 41 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index 05047192cb37..e2e85688ed3c 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -639,20 +639,54 @@ static void _dpu_plane_setup_scaler(struct 
>> dpu_sw_pipe *pipe,
>>                   fmt);
>>   }
>> +static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
>> +        struct dpu_sw_pipe *pipe,
>> +        struct dpu_hw_sspp_cfg *old_pipe_cfg,
> 
> Why is this called old_pipe_cfg instead of just pipe_cfg?

Ack. Probably got that wrong during mass-renaming and then missed to fix it.

> 
> 
>> +        u32 fill_color,
>> +        const struct dpu_format *fmt)
>> +{
>> +    struct dpu_hw_sspp_cfg pipe_cfg;
>> +
>> +    /* update sspp */
>> +    if (!pipe->sspp->ops.setup_solidfill)
>> +        return 0;
> 
> You can just return from here and make this function void?

Of course.

> 
>> +
>> +    pipe->sspp->ops.setup_solidfill(pipe, fill_color);
>> +
>> +    /* override scaler/decimation if solid fill */
>> +    pipe_cfg.dst_rect = old_pipe_cfg->dst_rect;
>> +
>> +    pipe_cfg.src_rect.x1 = 0;
>> +    pipe_cfg.src_rect.y1 = 0;
>> +    pipe_cfg.src_rect.x2 =
>> +        drm_rect_width(&pipe_cfg.dst_rect);
>> +    pipe_cfg.src_rect.y2 =
>> +        drm_rect_height(&pipe_cfg.dst_rect);
>> +
>> +    if (pipe->sspp->ops.setup_format)
>> +        pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL);
>> +
>> +    if (pipe->sspp->ops.setup_rects)
>> +        pipe->sspp->ops.setup_rects(pipe, &pipe_cfg);
>> +
>> +    _dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, 
>> pstate->rotation);
>> +
>> +    return 0;
>> +}
>> +
>>   /**
>>    * _dpu_plane_color_fill - enables color fill on plane
>>    * @pdpu:   Pointer to DPU plane object
>>    * @color:  RGB fill color value, [23..16] Blue, [15..8] Green, 
>> [7..0] Red
>>    * @alpha:  8-bit fill alpha value, 255 selects 100% alpha
>> - * Returns: 0 on success
>>    */
>> -static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
>> +static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
>>           uint32_t color, uint32_t alpha)
>>   {
>>       const struct dpu_format *fmt;
>>       const struct drm_plane *plane = &pdpu->base;
>>       struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
>> -    struct dpu_hw_sspp_cfg pipe_cfg;
>> +    u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24);
>>       DPU_DEBUG_PLANE(pdpu, "\n");
>> @@ -661,34 +695,12 @@ static int _dpu_plane_color_fill(struct 
>> dpu_plane *pdpu,
>>        * h/w only supports RGB variants
>>        */
>>       fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888);
>> +    /* should not happen ever */
>> +    if (!fmt)
>> +        return;
>>       /* update sspp */
>> -    if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
>> -        pstate->pipe.sspp->ops.setup_solidfill(&pstate->pipe,
>> -                (color & 0xFFFFFF) | ((alpha & 0xFF) << 24));
>> -
>> -        /* override scaler/decimation if solid fill */
>> -        pipe_cfg.dst_rect = pstate->base.dst;
>> -
>> -        pipe_cfg.src_rect.x1 = 0;
>> -        pipe_cfg.src_rect.y1 = 0;
>> -        pipe_cfg.src_rect.x2 =
>> -            drm_rect_width(&pipe_cfg.dst_rect);
>> -        pipe_cfg.src_rect.y2 =
>> -            drm_rect_height(&pipe_cfg.dst_rect);
>> -
>> -        if (pstate->pipe.sspp->ops.setup_format)
>> -            pstate->pipe.sspp->ops.setup_format(&pstate->pipe,
>> -                    fmt, DPU_SSPP_SOLID_FILL);
>> -
>> -        if (pstate->pipe.sspp->ops.setup_rects)
>> -            pstate->pipe.sspp->ops.setup_rects(&pstate->pipe,
>> -                    &pipe_cfg);
>> -
>> -        _dpu_plane_setup_scaler(&pstate->pipe, fmt, true, &pipe_cfg, 
>> pstate->rotation);
>> -    }
>> -
>> -    return 0;
>> +    _dpu_plane_color_fill_pipe(pstate, &pstate->pipe, 
>> &pstate->pipe_cfg, fill_color, fmt);
>>   }
>>   int dpu_plane_validate_multirect_v2(struct 
>> dpu_multirect_plane_states *plane)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 05047192cb37..e2e85688ed3c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -639,20 +639,54 @@  static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
 				fmt);
 }
 
+static int _dpu_plane_color_fill_pipe(struct dpu_plane_state *pstate,
+		struct dpu_sw_pipe *pipe,
+		struct dpu_hw_sspp_cfg *old_pipe_cfg,
+		u32 fill_color,
+		const struct dpu_format *fmt)
+{
+	struct dpu_hw_sspp_cfg pipe_cfg;
+
+	/* update sspp */
+	if (!pipe->sspp->ops.setup_solidfill)
+		return 0;
+
+	pipe->sspp->ops.setup_solidfill(pipe, fill_color);
+
+	/* override scaler/decimation if solid fill */
+	pipe_cfg.dst_rect = old_pipe_cfg->dst_rect;
+
+	pipe_cfg.src_rect.x1 = 0;
+	pipe_cfg.src_rect.y1 = 0;
+	pipe_cfg.src_rect.x2 =
+		drm_rect_width(&pipe_cfg.dst_rect);
+	pipe_cfg.src_rect.y2 =
+		drm_rect_height(&pipe_cfg.dst_rect);
+
+	if (pipe->sspp->ops.setup_format)
+		pipe->sspp->ops.setup_format(pipe, fmt, DPU_SSPP_SOLID_FILL);
+
+	if (pipe->sspp->ops.setup_rects)
+		pipe->sspp->ops.setup_rects(pipe, &pipe_cfg);
+
+	_dpu_plane_setup_scaler(pipe, fmt, true, &pipe_cfg, pstate->rotation);
+
+	return 0;
+}
+
 /**
  * _dpu_plane_color_fill - enables color fill on plane
  * @pdpu:   Pointer to DPU plane object
  * @color:  RGB fill color value, [23..16] Blue, [15..8] Green, [7..0] Red
  * @alpha:  8-bit fill alpha value, 255 selects 100% alpha
- * Returns: 0 on success
  */
-static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
+static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
 		uint32_t color, uint32_t alpha)
 {
 	const struct dpu_format *fmt;
 	const struct drm_plane *plane = &pdpu->base;
 	struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
-	struct dpu_hw_sspp_cfg pipe_cfg;
+	u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24);
 
 	DPU_DEBUG_PLANE(pdpu, "\n");
 
@@ -661,34 +695,12 @@  static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
 	 * h/w only supports RGB variants
 	 */
 	fmt = dpu_get_dpu_format(DRM_FORMAT_ABGR8888);
+	/* should not happen ever */
+	if (!fmt)
+		return;
 
 	/* update sspp */
-	if (fmt && pstate->pipe.sspp->ops.setup_solidfill) {
-		pstate->pipe.sspp->ops.setup_solidfill(&pstate->pipe,
-				(color & 0xFFFFFF) | ((alpha & 0xFF) << 24));
-
-		/* override scaler/decimation if solid fill */
-		pipe_cfg.dst_rect = pstate->base.dst;
-
-		pipe_cfg.src_rect.x1 = 0;
-		pipe_cfg.src_rect.y1 = 0;
-		pipe_cfg.src_rect.x2 =
-			drm_rect_width(&pipe_cfg.dst_rect);
-		pipe_cfg.src_rect.y2 =
-			drm_rect_height(&pipe_cfg.dst_rect);
-
-		if (pstate->pipe.sspp->ops.setup_format)
-			pstate->pipe.sspp->ops.setup_format(&pstate->pipe,
-					fmt, DPU_SSPP_SOLID_FILL);
-
-		if (pstate->pipe.sspp->ops.setup_rects)
-			pstate->pipe.sspp->ops.setup_rects(&pstate->pipe,
-					&pipe_cfg);
-
-		_dpu_plane_setup_scaler(&pstate->pipe, fmt, true, &pipe_cfg, pstate->rotation);
-	}
-
-	return 0;
+	_dpu_plane_color_fill_pipe(pstate, &pstate->pipe, &pstate->pipe_cfg, fill_color, fmt);
 }
 
 int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)