From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources
from dpu_crtc_assign_resources().
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
[quic_abhinavk@quicinc.com: cleaned up formatting]
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
[quic_jesszhan@quicinc.com: dropped clearing num_mixers in CRTC disable
path]
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 32 ++++++++++++++++++++++++++---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--------------------
2 files changed, 31 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 b918c80d30b3..d53e986eee54 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1091,9 +1091,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
dpu_core_perf_crtc_update(crtc, 0);
- memset(cstate->mixers, 0, sizeof(cstate->mixers));
- cstate->num_mixers = 0;
-
/* disable clk & bw control until clk & bw properties are set */
cstate->bw_control = false;
cstate->bw_split_vote = false;
@@ -1164,6 +1161,7 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
}
#define MAX_HDISPLAY_SPLIT 1080
+#define MAX_CHANNELS_PER_CRTC 2
static struct msm_display_topology dpu_crtc_get_topology(
struct drm_crtc *crtc,
@@ -1208,9 +1206,14 @@ static struct msm_display_topology dpu_crtc_get_topology(
static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state)
{
+ struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_CRTC];
+ struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_CRTC];
+ struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_CRTC];
+ int i, num_lm, num_ctl, num_dspp;
struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
struct dpu_global_state *global_state;
struct msm_display_topology topology;
+ struct dpu_crtc_state *cstate;
int ret;
/*
@@ -1232,6 +1235,29 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_stat
if (ret)
return ret;
+ cstate = to_dpu_crtc_state(crtc_state);
+
+ num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+ crtc, DPU_HW_BLK_CTL, hw_ctl,
+ ARRAY_SIZE(hw_ctl));
+ num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+ crtc, DPU_HW_BLK_LM, hw_lm,
+ ARRAY_SIZE(hw_lm));
+ num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+ crtc, DPU_HW_BLK_DSPP, hw_dspp,
+ ARRAY_SIZE(hw_dspp));
+
+ for (i = 0; i < num_lm; i++) {
+ int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
+
+ cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
+ cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
+ if (i < num_dspp)
+ cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
+ }
+
+ cstate->num_mixers = num_lm;
+
return 0;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index ada9119326ca..36b677cf9c7a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1049,14 +1049,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
struct dpu_encoder_virt *dpu_enc;
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms;
- struct dpu_crtc_state *cstate;
struct dpu_global_state *global_state;
struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
- struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
- struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
- int num_lm, num_ctl, num_pp, num_dsc;
+ int num_pp, num_dsc;
unsigned int dsc_mask = 0;
int i;
@@ -1083,13 +1080,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp,
ARRAY_SIZE(hw_pp));
- num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
- drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
- num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
- drm_enc->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
- drm_enc->crtc, DPU_HW_BLK_DSPP, hw_dspp,
- ARRAY_SIZE(hw_dspp));
+ drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
@@ -1115,18 +1107,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
}
- cstate = to_dpu_crtc_state(crtc_state);
-
- for (i = 0; i < num_lm; i++) {
- int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
-
- cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
- cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
- cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
- }
-
- cstate->num_mixers = num_lm;
-
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
--
2.34.1On Tue, Sep 24, 2024 at 03:59:22PM GMT, Jessica Zhang wrote:
> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources
> from dpu_crtc_assign_resources().
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> [quic_abhinavk@quicinc.com: cleaned up formatting]
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> [quic_jesszhan@quicinc.com: dropped clearing num_mixers in CRTC disable
> path]
Same comment as before: the code is still there.
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 32 ++++++++++++++++++++++++++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--------------------
> 2 files changed, 31 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 b918c80d30b3..d53e986eee54 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1091,9 +1091,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
>
> dpu_core_perf_crtc_update(crtc, 0);
>
> - memset(cstate->mixers, 0, sizeof(cstate->mixers));
> - cstate->num_mixers = 0;
> -
> /* disable clk & bw control until clk & bw properties are set */
> cstate->bw_control = false;
> cstate->bw_split_vote = false;
> @@ -1164,6 +1161,7 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
> }
>
> #define MAX_HDISPLAY_SPLIT 1080
> +#define MAX_CHANNELS_PER_CRTC 2
>
> static struct msm_display_topology dpu_crtc_get_topology(
> struct drm_crtc *crtc,
> @@ -1208,9 +1206,14 @@ static struct msm_display_topology dpu_crtc_get_topology(
>
> static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state)
> {
> + struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_CRTC];
> + struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_CRTC];
> + struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_CRTC];
> + int i, num_lm, num_ctl, num_dspp;
> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> struct dpu_global_state *global_state;
> struct msm_display_topology topology;
> + struct dpu_crtc_state *cstate;
> int ret;
>
> /*
> @@ -1232,6 +1235,29 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_stat
> if (ret)
> return ret;
>
> + cstate = to_dpu_crtc_state(crtc_state);
> +
> + num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> + crtc, DPU_HW_BLK_CTL, hw_ctl,
> + ARRAY_SIZE(hw_ctl));
> + num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> + crtc, DPU_HW_BLK_LM, hw_lm,
> + ARRAY_SIZE(hw_lm));
> + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> + crtc, DPU_HW_BLK_DSPP, hw_dspp,
> + ARRAY_SIZE(hw_dspp));
> +
> + for (i = 0; i < num_lm; i++) {
> + int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> +
> + cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
> + cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
> + if (i < num_dspp)
> + cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
> + }
> +
> + cstate->num_mixers = num_lm;
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ada9119326ca..36b677cf9c7a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1049,14 +1049,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> struct dpu_encoder_virt *dpu_enc;
> struct msm_drm_private *priv;
> struct dpu_kms *dpu_kms;
> - struct dpu_crtc_state *cstate;
> struct dpu_global_state *global_state;
> struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
> - struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
> - struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
> struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> - int num_lm, num_ctl, num_pp, num_dsc;
> + int num_pp, num_dsc;
> unsigned int dsc_mask = 0;
> int i;
>
> @@ -1083,13 +1080,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp,
> ARRAY_SIZE(hw_pp));
> - num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
> - num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> - drm_enc->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
> dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> - drm_enc->crtc, DPU_HW_BLK_DSPP, hw_dspp,
> - ARRAY_SIZE(hw_dspp));
> + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
Please don't mix reindentation with the actual changes. It makes it
harder to read.
>
> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
> @@ -1115,18 +1107,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
> }
>
> - cstate = to_dpu_crtc_state(crtc_state);
> -
> - for (i = 0; i < num_lm; i++) {
> - int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> -
> - cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
> - cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
> - cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
> - }
> -
> - cstate->num_mixers = num_lm;
> -
> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On 9/24/2024 4:16 PM, Dmitry Baryshkov wrote:
> On Tue, Sep 24, 2024 at 03:59:22PM GMT, Jessica Zhang wrote:
>> From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>
>> Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources
>> from dpu_crtc_assign_resources().
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> [quic_abhinavk@quicinc.com: cleaned up formatting]
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> [quic_jesszhan@quicinc.com: dropped clearing num_mixers in CRTC disable
>> path]
>
> Same comment as before: the code is still there.
Hi Dmitry,
I thought the original comment was to move the dropping `num_mixers = 0`
chunk from the previous patch to this one?
Sorry if I misunderstood.
Thanks,
Jessica Zhang
>
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 32 ++++++++++++++++++++++++++---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--------------------
>> 2 files changed, 31 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 b918c80d30b3..d53e986eee54 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -1091,9 +1091,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
>>
>> dpu_core_perf_crtc_update(crtc, 0);
>>
>> - memset(cstate->mixers, 0, sizeof(cstate->mixers));
>> - cstate->num_mixers = 0;
>> -
>> /* disable clk & bw control until clk & bw properties are set */
>> cstate->bw_control = false;
>> cstate->bw_split_vote = false;
>> @@ -1164,6 +1161,7 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
>> }
>>
>> #define MAX_HDISPLAY_SPLIT 1080
>> +#define MAX_CHANNELS_PER_CRTC 2
>>
>> static struct msm_display_topology dpu_crtc_get_topology(
>> struct drm_crtc *crtc,
>> @@ -1208,9 +1206,14 @@ static struct msm_display_topology dpu_crtc_get_topology(
>>
>> static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state)
>> {
>> + struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_CRTC];
>> + struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_CRTC];
>> + struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_CRTC];
>> + int i, num_lm, num_ctl, num_dspp;
>> struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
>> struct dpu_global_state *global_state;
>> struct msm_display_topology topology;
>> + struct dpu_crtc_state *cstate;
>> int ret;
>>
>> /*
>> @@ -1232,6 +1235,29 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_stat
>> if (ret)
>> return ret;
>>
>> + cstate = to_dpu_crtc_state(crtc_state);
>> +
>> + num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> + crtc, DPU_HW_BLK_CTL, hw_ctl,
>> + ARRAY_SIZE(hw_ctl));
>> + num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> + crtc, DPU_HW_BLK_LM, hw_lm,
>> + ARRAY_SIZE(hw_lm));
>> + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> + crtc, DPU_HW_BLK_DSPP, hw_dspp,
>> + ARRAY_SIZE(hw_dspp));
>> +
>> + for (i = 0; i < num_lm; i++) {
>> + int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
>> +
>> + cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
>> + cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
>> + if (i < num_dspp)
>> + cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
>> + }
>> +
>> + cstate->num_mixers = num_lm;
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index ada9119326ca..36b677cf9c7a 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -1049,14 +1049,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>> struct dpu_encoder_virt *dpu_enc;
>> struct msm_drm_private *priv;
>> struct dpu_kms *dpu_kms;
>> - struct dpu_crtc_state *cstate;
>> struct dpu_global_state *global_state;
>> struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC];
>> struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
>> - struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
>> - struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
>> struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
>> - int num_lm, num_ctl, num_pp, num_dsc;
>> + int num_pp, num_dsc;
>> unsigned int dsc_mask = 0;
>> int i;
>>
>> @@ -1083,13 +1080,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>> num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp,
>> ARRAY_SIZE(hw_pp));
>> - num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
>> - num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> - drm_enc->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
>> dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>> - drm_enc->crtc, DPU_HW_BLK_DSPP, hw_dspp,
>> - ARRAY_SIZE(hw_dspp));
>> + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
>
> Please don't mix reindentation with the actual changes. It makes it
> harder to read.
>
>>
>> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>> dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
>> @@ -1115,18 +1107,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>> dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
>> }
>>
>> - cstate = to_dpu_crtc_state(crtc_state);
>> -
>> - for (i = 0; i < num_lm; i++) {
>> - int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
>> -
>> - cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
>> - cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
>> - cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
>> - }
>> -
>> - cstate->num_mixers = num_lm;
>> -
>> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
>> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>>
>>
>> --
>> 2.34.1
>>
>
> --
> With best wishes
> Dmitry
On Tue, Sep 24, 2024 at 05:37:30PM GMT, Jessica Zhang wrote:
>
>
> On 9/24/2024 4:16 PM, Dmitry Baryshkov wrote:
> > On Tue, Sep 24, 2024 at 03:59:22PM GMT, Jessica Zhang wrote:
> > > From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >
> > > Stop poking into CRTC state from dpu_encoder.c, fill CRTC HW resources
> > > from dpu_crtc_assign_resources().
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > [quic_abhinavk@quicinc.com: cleaned up formatting]
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> > > [quic_jesszhan@quicinc.com: dropped clearing num_mixers in CRTC disable
> > > path]
> >
> > Same comment as before: the code is still there.
>
> Hi Dmitry,
>
> I thought the original comment was to move the dropping `num_mixers = 0`
> chunk from the previous patch to this one?
>
> Sorry if I misunderstood.
Code move is fine. The commit message should reflect the code though. So
if you have "dropped clearing num_mixers..." in the commit message, it
is surprising to see the code actually claring num_mixers.
>
> Thanks,
>
> Jessica Zhang
>
> >
> > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 32 ++++++++++++++++++++++++++---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++--------------------
> > > 2 files changed, 31 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 b918c80d30b3..d53e986eee54 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -1091,9 +1091,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
> > > dpu_core_perf_crtc_update(crtc, 0);
> > > - memset(cstate->mixers, 0, sizeof(cstate->mixers));
> > > - cstate->num_mixers = 0;
> > > -
> > > /* disable clk & bw control until clk & bw properties are set */
> > > cstate->bw_control = false;
> > > cstate->bw_split_vote = false;
> > > @@ -1164,6 +1161,7 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
> > > }
> > > #define MAX_HDISPLAY_SPLIT 1080
> > > +#define MAX_CHANNELS_PER_CRTC 2
> > > static struct msm_display_topology dpu_crtc_get_topology(
> > > struct drm_crtc *crtc,
> > > @@ -1208,9 +1206,14 @@ static struct msm_display_topology dpu_crtc_get_topology(
> > > static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_state *crtc_state)
> > > {
> > > + struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_CRTC];
> > > + struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_CRTC];
> > > + struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_CRTC];
> > > + int i, num_lm, num_ctl, num_dspp;
> > > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> > > struct dpu_global_state *global_state;
> > > struct msm_display_topology topology;
> > > + struct dpu_crtc_state *cstate;
> > > int ret;
> > > /*
> > > @@ -1232,6 +1235,29 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc, struct drm_crtc_stat
> > > if (ret)
> > > return ret;
> > > + cstate = to_dpu_crtc_state(crtc_state);
> > > +
> > > + num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > + crtc, DPU_HW_BLK_CTL, hw_ctl,
> > > + ARRAY_SIZE(hw_ctl));
> > > + num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > + crtc, DPU_HW_BLK_LM, hw_lm,
> > > + ARRAY_SIZE(hw_lm));
> > > + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > + crtc, DPU_HW_BLK_DSPP, hw_dspp,
> > > + ARRAY_SIZE(hw_dspp));
> > > +
> > > + for (i = 0; i < num_lm; i++) {
> > > + int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> > > +
> > > + cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
> > > + cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
> > > + if (i < num_dspp)
> > > + cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
> > > + }
> > > +
> > > + cstate->num_mixers = num_lm;
> > > +
> > > return 0;
> > > }
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index ada9119326ca..36b677cf9c7a 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -1049,14 +1049,11 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> > > struct dpu_encoder_virt *dpu_enc;
> > > struct msm_drm_private *priv;
> > > struct dpu_kms *dpu_kms;
> > > - struct dpu_crtc_state *cstate;
> > > struct dpu_global_state *global_state;
> > > struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC];
> > > struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
> > > - struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
> > > - struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
> > > struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
> > > - int num_lm, num_ctl, num_pp, num_dsc;
> > > + int num_pp, num_dsc;
> > > unsigned int dsc_mask = 0;
> > > int i;
> > > @@ -1083,13 +1080,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> > > num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > drm_enc->crtc, DPU_HW_BLK_PINGPONG, hw_pp,
> > > ARRAY_SIZE(hw_pp));
> > > - num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > - drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
> > > - num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > - drm_enc->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
> > > dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> > > - drm_enc->crtc, DPU_HW_BLK_DSPP, hw_dspp,
> > > - ARRAY_SIZE(hw_dspp));
> > > + drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
> >
> > Please don't mix reindentation with the actual changes. It makes it
> > harder to read.
> >
> > > for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> > > dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
> > > @@ -1115,18 +1107,6 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> > > dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
> > > }
> > > - cstate = to_dpu_crtc_state(crtc_state);
> > > -
> > > - for (i = 0; i < num_lm; i++) {
> > > - int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> > > -
> > > - cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
> > > - cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
> > > - cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
> > > - }
> > > -
> > > - cstate->num_mixers = num_lm;
> > > -
> > > for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> > > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
>
--
With best wishes
Dmitry
© 2016 - 2024 Red Hat, Inc.