This series will enable color features on sc7280 target which has
primary panel as eDP
The series removes DSPP allocation based on encoder type and allows
the DSPP reservation based on user request via CTM.
The series will release/reserve the dpu resources when ever there is
a topology change to suit the new requirements.
Kalyan Thota (4):
drm/msm/dpu: clear DSPP reservations in rm release
drm/msm/dpu: add DSPPs into reservation upon a CTM request
drm/msm/dpu: avoid unnecessary check in DPU reservations
drm/msm/dpu: reserve the resources on topology change
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 61 +++++++++++++++++------------
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 2 +
3 files changed, 40 insertions(+), 25 deletions(-)
--
2.7.4
Return immediately on failure, this will make dpu reservations
part look cleaner.
Signed-off-by: Kalyan Thota <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 46d2a5c..3920efd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -636,25 +636,22 @@ static int dpu_encoder_virt_atomic_check(
if (ret) {
DPU_ERROR_ENC(dpu_enc,
"mode unsupported, phys idx %d\n", i);
- break;
+ return ret;
}
}
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
- /* Reserve dynamic resources now. */
- if (!ret) {
- /*
- * Release and Allocate resources on every modeset
- * Dont allocate when active is false.
- */
- if (drm_atomic_crtc_needs_modeset(crtc_state)) {
- dpu_rm_release(global_state, drm_enc);
+ /*
+ * Release and Allocate resources on every modeset
+ * Dont allocate when active is false.
+ */
+ if (drm_atomic_crtc_needs_modeset(crtc_state)) {
+ dpu_rm_release(global_state, drm_enc);
- if (!crtc_state->active_changed || crtc_state->active)
- ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
- drm_enc, crtc_state, topology);
- }
+ if (!crtc_state->active_changed || crtc_state->active)
+ ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
+ drm_enc, crtc_state, topology);
}
trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
--
2.7.4
Some features like CTM can be enabled dynamically. Release
and reserve the DPU resources whenever a topology change
occurs such that required hw blocks are allocated appropriately.
Signed-off-by: Kalyan Thota <[email protected]>
---
Changes in v1:
- Avoid mode_set call directly (Dmitry)
Changes in v2:
- Minor nits (Dmitry)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 ++++++++++++++++++++++-----
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 539b68b..85bd5645 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -204,6 +204,7 @@ struct dpu_crtc {
* @hw_ctls : List of active ctl paths
* @crc_source : CRC source
* @crc_frame_skip_count: Number of frames skipped before getting CRC
+ * @ctm_enabled : Cached color management enablement state
*/
struct dpu_crtc_state {
struct drm_crtc_state base;
@@ -225,6 +226,7 @@ struct dpu_crtc_state {
enum dpu_crtc_crc_source crc_source;
int crc_frame_skip_count;
+ bool ctm_enabled;
};
#define to_dpu_crtc_state(x) \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3920efd..7bb4840 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -217,6 +217,22 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
};
+static bool _dpu_enc_is_dspp_changed(struct drm_crtc_state *crtc_state,
+ struct msm_display_topology topology)
+{
+ struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
+
+ if (drm_atomic_crtc_needs_modeset(crtc_state))
+ return true;
+
+ if ((cstate->ctm_enabled && !topology.num_dspp) ||
+ (!cstate->ctm_enabled && topology.num_dspp)) {
+ crtc_state->mode_changed = true;
+ return true;
+ }
+
+ return false;
+}
bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
{
@@ -642,14 +658,15 @@ static int dpu_encoder_virt_atomic_check(
topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
+ _dpu_enc_is_dspp_changed(crtc_state, topology);
+
/*
* Release and Allocate resources on every modeset
- * Dont allocate when active is false.
*/
if (drm_atomic_crtc_needs_modeset(crtc_state)) {
dpu_rm_release(global_state, drm_enc);
- if (!crtc_state->active_changed || crtc_state->active)
+ if (crtc_state->enable)
ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
drm_enc, crtc_state, topology);
}
@@ -1022,7 +1039,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_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_lm, num_ctl, num_pp, num_dsc, num_dspp;
unsigned int dsc_mask = 0;
int i;
@@ -1053,7 +1070,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
drm_enc->base.id, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
- dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+ num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
ARRAY_SIZE(hw_dspp));
@@ -1084,7 +1101,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
}
cstate->num_mixers = num_lm;
-
+ cstate->ctm_enabled = !!num_dspp;
dpu_enc->connector = conn_state->connector;
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
--
2.7.4
On 07/02/2023 16:29, Kalyan Thota wrote:
> Return immediately on failure, this will make dpu reservations
> part look cleaner.
>
> Signed-off-by: Kalyan Thota <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On 07/02/2023 16:29, Kalyan Thota wrote:
> Some features like CTM can be enabled dynamically. Release
> and reserve the DPU resources whenever a topology change
> occurs such that required hw blocks are allocated appropriately.
>
> Signed-off-by: Kalyan Thota <[email protected]>
> ---
> Changes in v1:
> - Avoid mode_set call directly (Dmitry)
>
> Changes in v2:
> - Minor nits (Dmitry)
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 ++++++++++++++++++++++-----
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 539b68b..85bd5645 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -204,6 +204,7 @@ struct dpu_crtc {
> * @hw_ctls : List of active ctl paths
> * @crc_source : CRC source
> * @crc_frame_skip_count: Number of frames skipped before getting CRC
> + * @ctm_enabled : Cached color management enablement state
> */
> struct dpu_crtc_state {
> struct drm_crtc_state base;
> @@ -225,6 +226,7 @@ struct dpu_crtc_state {
>
> enum dpu_crtc_crc_source crc_source;
> int crc_frame_skip_count;
> + bool ctm_enabled;
> };
>
> #define to_dpu_crtc_state(x) \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 3920efd..7bb4840 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -217,6 +217,22 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
> 15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
> };
>
> +static bool _dpu_enc_is_dspp_changed(struct drm_crtc_state *crtc_state,
> + struct msm_display_topology topology)
> +{
> + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
> +
> + if (drm_atomic_crtc_needs_modeset(crtc_state))
> + return true;
I think this check doesn't belong to the is_dspp_changed() function.
> +
> + if ((cstate->ctm_enabled && !topology.num_dspp) ||
> + (!cstate->ctm_enabled && topology.num_dspp)) {
> + crtc_state->mode_changed = true;
> + return true;
> + }
> +
> + return false;
> +}
>
> bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
> {
> @@ -642,14 +658,15 @@ static int dpu_encoder_virt_atomic_check(
>
> topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
>
> + _dpu_enc_is_dspp_changed(crtc_state, topology);
> +
> /*
> * Release and Allocate resources on every modeset
> - * Dont allocate when active is false.
> */
> if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> dpu_rm_release(global_state, drm_enc);
>
> - if (!crtc_state->active_changed || crtc_state->active)
> + if (crtc_state->enable)
> ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
> drm_enc, crtc_state, topology);
> }
> @@ -1022,7 +1039,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_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_lm, num_ctl, num_pp, num_dsc, num_dspp;
> unsigned int dsc_mask = 0;
> int i;
>
> @@ -1053,7 +1070,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> drm_enc->base.id, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
> num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
> - dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> + num_dspp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
> drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
> ARRAY_SIZE(hw_dspp));
>
> @@ -1084,7 +1101,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
> }
>
> cstate->num_mixers = num_lm;
> -
> + cstate->ctm_enabled = !!num_dspp;
> dpu_enc->connector = conn_state->connector;
>
> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
--
With best wishes
Dmitry