[Freedreno] [PATCH 6/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()

[Abhinav Kumar]

On 5/2/2023 4:58 PM, Dmitry Baryshkov wrote:
> On 03/05/2023 02:57, Abhinav Kumar wrote:
>>
>>
>> On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote:
>>> There is little sense to get intf index just to call dpu_rm_get_intf()
>>> on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 20 ++++++++------------
>>>   1 file changed, 8 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 507ff3f88c67..b35e92c658ad 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -1259,22 +1259,23 @@ static void 
>>> dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc,
>>>       mutex_unlock(&dpu_enc->enc_lock);
>>>   }
>>> -static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg 
>>> *catalog,
>>> +static struct dpu_hw_intf *dpu_encoder_get_intf(const struct 
>>> dpu_mdss_cfg *catalog,
>>> +        struct dpu_rm *dpu_rm,
>>>           enum dpu_intf_type type, u32 controller_id)
>>>   {
>>>       int i = 0;
>>>       if (type == INTF_WB)
>>> -        return INTF_MAX;
>>> +        return NULL;
>>>       for (i = 0; i < catalog->intf_count; i++) {
>>>           if (catalog->intf[i].type == type
>>>               && catalog->intf[i].controller_id == controller_id) {
>>> -            return catalog->intf[i].id;
>>> +            return dpu_rm_get_intf(dpu_rm, catalog->intf[i].id);
>>>           }
>>
>> Why has the for loop been retained in this function but not for 
>> writeback? is there any difference? Doesnt looks like there needs to be.
> 
> For writeback we always return controller_id (WB_2). For interfaces we 
> have to map type+controller_id to the INTF instance.

Ah correct, got it now. With that minor comment fixed from below,

Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>

> 
>>
>>>       }
>>> -    return INTF_MAX;
>>> +    return NULL;
>>>   }
>>>   void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
>>> @@ -2244,7 +2245,6 @@ static int dpu_encoder_setup_display(struct 
>>> dpu_encoder_virt *dpu_enc,
>>>            * h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right
>>>            */
>>>           u32 controller_id = disp_info->h_tile_instance[i];
>>> -        enum dpu_intf intf_idx;
>>>           if (disp_info->num_of_h_tiles > 1) {
>>>               if (i == 0)
>>> @@ -2258,12 +2258,9 @@ static int dpu_encoder_setup_display(struct 
>>> dpu_encoder_virt *dpu_enc,
>>>           DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
>>>                   i, controller_id, phys_params.split_role);
>>> -        intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
>>> -                                disp_info->intf_type,
>>> -                                controller_id);
>>> -
>>> -        if (intf_idx >= INTF_0 && intf_idx < INTF_MAX)
>>> -            phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, 
>>> intf_idx);
>>> +        phys_params.hw_intf = dpu_encoder_get_intf(dpu_kms->catalog, 
>>> &dpu_kms->rm,
>>> +                               disp_info->intf_type,
>>> +                               controller_id);
>>>           if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX)
>>>               phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, 
>>> controller_id);
>>> @@ -2287,7 +2284,6 @@ static int dpu_encoder_setup_display(struct 
>>> dpu_encoder_virt *dpu_enc,
>>>               DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n");
>>>               break;
>>>           }
>>> -
>> unnecessary change?
> 
> 
> ack, it sneaked in. I'll drop it for v2.
> 
>>>       }
>>>       mutex_unlock(&dpu_enc->enc_lock);
>