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 <jsa...@codeaurora.org>
> ---
>  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.


> +                     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
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to