On 15/06/2022 19:11, Jessica Zhang wrote:
On 6/15/2022 2:35 AM, Dmitry Baryshkov wrote:
On Wed, 15 Jun 2022 at 00:13, Jessica Zhang
<quic_jesszhan@xxxxxxxxxxx> wrote:
Move layer mixer-specific section of dpu_crtc_get_crc() into a separate
helper method. This way, we can make it easier to get CRCs from other HW
blocks by adding other get_crc helper methods.
Changes since V1:
- Moved common bitmasks to dpu_hw_util.h
- Moved common CRC methods to dpu_hw_util.c
- Updated copyrights
- Changed crcs array to a dynamically allocated array and added it as a
member of crtc_state
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx>
Please split this into separate patches. One for hw_util, one for the
rest
Sure
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 88 +++++++++++++--------
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 4 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 42 +---------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 46 ++++++++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 16 ++++
5 files changed, 124 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b56f777dbd0e..16742a66878e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights
reserved.
* Copyright (c) 2014-2021 The Linux Foundation. All rights reserved.
* Copyright (C) 2013 Red Hat
* Author: Rob Clark <robdclark@xxxxxxxxx>
@@ -88,6 +89,11 @@ static int dpu_crtc_verify_crc_source(struct
drm_crtc *crtc,
enum dpu_crtc_crc_source source =
dpu_crtc_parse_crc_source(src_name);
struct dpu_crtc_state *crtc_state =
to_dpu_crtc_state(crtc->state);
+ if (crtc_state->crcs) {
+ kfree(crtc_state->crcs);
+ crtc_state->crcs = NULL;
+ }
+
if (source < 0) {
DRM_DEBUG_DRIVER("Invalid source %s for CRTC%d\n",
src_name, crtc->index);
return -EINVAL;
@@ -96,20 +102,37 @@ static int dpu_crtc_verify_crc_source(struct
drm_crtc *crtc,
if (source == DPU_CRTC_CRC_SOURCE_LAYER_MIXER)
*values_cnt = crtc_state->num_mixers;
+ crtc_state->crcs = kcalloc(*values_cnt,
sizeof(crtc_state->crcs), GFP_KERNEL);
+
I do not quite like the idea of constantly allocating and freeing the
crcs array. I'd suggest defining sensible MAX_CRC_VALUES, using a
static array and verifying that no one over commits the
MAX_CRC_VALUES.
If at some point we decide that we really need the dynamic array, we
can implement it later. We already had dynamic arrays and Rob had to
fix it. See 00326bfa4e63 ("drm/msm/dpu: Remove dynamic allocation from
atomic context").
Ah, got it... the reason I used a dynamic array here was because AFAIK
we don't have a defined upper limit for how many drm_encoders can be
connected to a CRTC simultaneously. Do you have a suggestion on what
value we can set for MAX_CRC_VALUES?
Three? Two for two hw_intfs?
--
With best wishes
Dmitry
