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

[Dmitry Baryshkov]

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.

> 
>>       }
>> -    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);

-- 
With best wishes
Dmitry