Re: [Freedreno] [PATCH 01/25] drm/msm/dpu: fix hw ctl retrieval for mixer muxing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-10-09 11:07, Sean Paul wrote:
On Mon, Oct 08, 2018 at 09:27:18PM -0700, Jeykumar Sankaran wrote:
Layer mixer/pingpong block counts and hw ctl block counts
will not be same for all the topologies (e.g. layer
mixer muxing to single interface)

Use the encoder's split_role info to retrieve the
respective control path for programming.

Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 96cdf06..d12f896 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1060,6 +1060,7 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,

 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
 		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+		int ctl_index;

 		if (phys) {
 			if (!dpu_enc->hw_pp[i]) {
@@ -1068,14 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,
 				return;
 			}

-			if (!hw_ctl[i]) {
+			ctl_index = phys->split_role == ENC_ROLE_SLAVE ? 1
: 0;
+

What if MAX_CHANNELS_PER_ENC isn't 2? Similarly, what if num_phys_encs >
MAX_CHANNELS_PER_ENC? It seems like there should be a more formal
relationship
between all of these verious values (num_of_h_tiles assumed to be <= 2 as
well).
If one of them changes beyond the assumed bound, the rest of the driver
falls
over pretty hard.

MAX_CHANNELS_PER_ENC is set to 2 to represent HW limitation on the chipset as we cannot gang up more than 2 LM chain to an interface. Supporting more than 2
might demand much larger changes than validating for boundaries.

num_phys_enc is the max no of phys encoders we create as we are looping through
num_of_h_tiles which cannot be more than priv->dsi array size.

So its very unlikely we would expect these loops to go out of bound!

Thanks,
Jeykumar S.

+			if (!hw_ctl[ctl_index]) {
 				DPU_ERROR_ENC(dpu_enc, "no ctl block
assigned"
-					     "at idx: %d\n", i);
+					     "at idx: %d\n", ctl_index);
 				return;

When you return on error here, should you give back the resources that
you've
already provisioned?

 			}

 			phys->hw_pp = dpu_enc->hw_pp[i];
-			phys->hw_ctl = hw_ctl[i];
+			phys->hw_ctl = hw_ctl[ctl_index];

 			phys->connector = conn->state->connector;
 			if (phys->ops.mode_set)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/freedreno

--
Jeykumar S



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux