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.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24 ++++++++++++++++++------
1 file changed, 18 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 114c803ff99b..95fc0394d13e 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,20 @@ 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_bgr_fill_color(struct drm_solid_fill solid_fill)
+{
+ uint32_t ret = 0;
+ uint8_t b = solid_fill.b >> 24;
+ uint8_t g = solid_fill.g >> 24;
+ uint8_t r = solid_fill.r >> 24;
+
+ ret |= b << 16;
+ ret |= g << 8;
+ ret |= r;
+
+ return ret;
+}
+
/**
* _dpu_plane_color_fill - enables color fill on plane
* @pdpu: Pointer to DPU plane object
@@ -977,9 +989,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, 0xFFFFFF, 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_bgr_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 +1036,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 */
--
2.41.0On 28/07/2023 20:02, 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.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24 ++++++++++++++++++------
> 1 file changed, 18 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 114c803ff99b..95fc0394d13e 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,20 @@ 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_bgr_fill_color(struct drm_solid_fill solid_fill)
As I commented for v4 (please excuse me for not responding to your email
at thattime), we can return abgr here, taking plane->state->alpha into
account.
> +{
> + uint32_t ret = 0;
> + uint8_t b = solid_fill.b >> 24;
> + uint8_t g = solid_fill.g >> 24;
> + uint8_t r = solid_fill.r >> 24;
> +
> + ret |= b << 16;
> + ret |= g << 8;
> + ret |= r;
> +
> + return ret;
> +}
> +
> /**
> * _dpu_plane_color_fill - enables color fill on plane
> * @pdpu: Pointer to DPU plane object
> @@ -977,9 +989,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, 0xFFFFFF, 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_bgr_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 +1036,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
On 7/30/2023 9:15 PM, Dmitry Baryshkov wrote:
> On 28/07/2023 20:02, 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.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24 ++++++++++++++++++------
>> 1 file changed, 18 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 114c803ff99b..95fc0394d13e 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,20 @@ 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_bgr_fill_color(struct drm_solid_fill
>> solid_fill)
>
> As I commented for v4 (please excuse me for not responding to your email
> at thattime), we can return abgr here, taking plane->state->alpha into
> account.
Hi Dmitry,
Since it seems that this comment wasn't resolved, I can drop your R-B
tag in the next revision.
From my previous response, I pointed out that the color parameter
expects an RGB value [1].
So is the intention here to refactor _dpu_plane_color_fill() to accept
an ABGR8888 color?
Thanks,
Jessica Zhang
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L676
>
>> +{
>> + uint32_t ret = 0;
>> + uint8_t b = solid_fill.b >> 24;
>> + uint8_t g = solid_fill.g >> 24;
>> + uint8_t r = solid_fill.r >> 24;
>> +
>> + ret |= b << 16;
>> + ret |= g << 8;
>> + ret |= r;
>> +
>> + return ret;
>> +}
>> +
>> /**
>> * _dpu_plane_color_fill - enables color fill on plane
>> * @pdpu: Pointer to DPU plane object
>> @@ -977,9 +989,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, 0xFFFFFF, 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_bgr_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 +1036,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
>
On 01/08/2023 03:39, Jessica Zhang wrote:
>
>
> On 7/30/2023 9:15 PM, Dmitry Baryshkov wrote:
>> On 28/07/2023 20:02, 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.
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24
>>> ++++++++++++++++++------
>>> 1 file changed, 18 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 114c803ff99b..95fc0394d13e 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,20 @@ 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_bgr_fill_color(struct drm_solid_fill
>>> solid_fill)
>>
>> As I commented for v4 (please excuse me for not responding to your
>> email at thattime), we can return abgr here, taking
>> plane->state->alpha into account.
>
> Hi Dmitry,
>
> Since it seems that this comment wasn't resolved, I can drop your R-B
> tag in the next revision.
It's a minor issue, so no need to drop the tag.
>
> From my previous response, I pointed out that the color parameter
> expects an RGB value [1].
>
> So is the intention here to refactor _dpu_plane_color_fill() to accept
> an ABGR8888 color?
That's what I'm suggesting.
>
> Thanks,
>
> Jessica Zhang
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L676
>
>>
>>> +{
>>> + uint32_t ret = 0;
>>> + uint8_t b = solid_fill.b >> 24;
>>> + uint8_t g = solid_fill.g >> 24;
>>> + uint8_t r = solid_fill.r >> 24;
>>> +
>>> + ret |= b << 16;
>>> + ret |= g << 8;
>>> + ret |= r;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> /**
>>> * _dpu_plane_color_fill - enables color fill on plane
>>> * @pdpu: Pointer to DPU plane object
>>> @@ -977,9 +989,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, 0xFFFFFF, 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_bgr_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 +1036,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
>>
--
With best wishes
Dmitry
On 7/31/2023 5:52 PM, Dmitry Baryshkov wrote:
> On 01/08/2023 03:39, Jessica Zhang wrote:
>>
>>
>> On 7/30/2023 9:15 PM, Dmitry Baryshkov wrote:
>>> On 28/07/2023 20:02, 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.
>>>>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>>>> ---
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24
>>>> ++++++++++++++++++------
>>>> 1 file changed, 18 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 114c803ff99b..95fc0394d13e 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,20 @@ 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_bgr_fill_color(struct drm_solid_fill
>>>> solid_fill)
>>>
>>> As I commented for v4 (please excuse me for not responding to your
>>> email at thattime), we can return abgr here, taking
>>> plane->state->alpha into account.
>>
>> Hi Dmitry,
>>
>> Since it seems that this comment wasn't resolved, I can drop your R-B
>> tag in the next revision.
>
> It's a minor issue, so no need to drop the tag.
>
>>
>> From my previous response, I pointed out that the color parameter
>> expects an RGB value [1].
>>
>> So is the intention here to refactor _dpu_plane_color_fill() to accept
>> an ABGR8888 color?
>
> That's what I'm suggesting.
Hi Dmitry,
Got it, sounds good to me.
Thanks,
Jessica Zhang
>
>>
>> Thanks,
>>
>> Jessica Zhang
>>
>> [1]
>> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L676
>>
>>>
>>>> +{
>>>> + uint32_t ret = 0;
>>>> + uint8_t b = solid_fill.b >> 24;
>>>> + uint8_t g = solid_fill.g >> 24;
>>>> + uint8_t r = solid_fill.r >> 24;
>>>> +
>>>> + ret |= b << 16;
>>>> + ret |= g << 8;
>>>> + ret |= r;
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> /**
>>>> * _dpu_plane_color_fill - enables color fill on plane
>>>> * @pdpu: Pointer to DPU plane object
>>>> @@ -977,9 +989,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, 0xFFFFFF, 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_bgr_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 +1036,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
>>>
>
> --
> With best wishes
> Dmitry
>
© 2016 - 2026 Red Hat, Inc.