[PATCH] drm/msm/dp: use ARRAY_SIZE for calculating num_descs

[Kuogee Hsieh]

On 6/27/2022 2:23 PM, Dmitry Baryshkov wrote:
> On 28/06/2022 00:01, Kuogee Hsieh wrote:
>>
>> On 6/27/2022 1:05 PM, Dmitry Baryshkov wrote:
>>> On Mon, 27 Jun 2022 at 22:26, Kuogee Hsieh <quic_khsieh at quicinc.com> 
>>> wrote:
>>>>
>>>> On 6/27/2022 9:54 AM, Dmitry Baryshkov wrote:
>>>>> If for some reason the msm_dp_config::descs array starts from 
>>>>> non-zero
>>>>> index or contains the hole, setting the msm_dp_config::num_descs 
>>>>> might
>>>>> be not that obvious and error-prone. Use ARRAY_SIZE to set this field
>>>>> rather than encoding the value manually.
>>>>>
>>>>> Reported-by: Kuogee Hsieh <quic_khsieh at quicinc.com>
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
Reviewed-by: Kuogee Hsieh <quic_khsieh at quicinc.com>
>>>>> ---
>>>>>    drivers/gpu/drm/msm/dp/dp_display.c | 46 
>>>>> +++++++++++++++++------------
>>>>>    1 file changed, 27 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>>>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>> index f87fa3ba1e25..6fed738a9467 100644
>>>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>>>> @@ -131,35 +131,43 @@ struct msm_dp_config {
>>>>>        size_t num_descs;
>>>>>    };
>>>>>
>>>>> +static const struct msm_dp_desc sc7180_dp_descs[] = {
>>>>> +     [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> +};
>>>>> +
>>>>>    static const struct msm_dp_config sc7180_dp_cfg = {
>>>>> -     .descs = (const struct msm_dp_desc[]) {
>>>>> -             [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> -     },
>>>>> -     .num_descs = 1,
>>>>> +     .descs = sc7180_dp_descs,
>>>>> +     .num_descs = ARRAY_SIZE(sc7180_dp_descs),
>>>>> +};
>>>>> +
>>>> why you want to do that?
>>>>
>>>> It is very clear only one entry, why you want to make it 2 entry here?
>>>>
>>>> can you just embedded MSM_DP_COTROLLER_x into struct msm_dp_config?
>>> Because we had enough stories of using a manually set 'number of
>>> something' field. So I'd prefer to have it done automatically.
>>> Also using the indexed array spares us from 'look for the DP
>>> controller number N' functions. You can just get it.
>>
>> static const struct msm_dp_config sc7280_dp_cfg = {
>>           .descs = (const struct msm_dp_desc[]) {
>>                   [MSM_DP_CONTROLLER_1] = { .io_start = 0x0aea0000, 
>> .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
>>           },
>>           .num_descs = ARRAY_SIZE(sc7280_dp_descs),
>
> This will not work.
>
>> };
>>
>> At above example table, it just waste one entry. is it ok?
>
> I'd assume this is an interim development state. Then it's fine. In 
> the final version one would fill all existing DP controllers starting 
> from 0.
>
>>
>> can you elaborate  more on 'look for the DP controller number N' 
>> functions, where is it?
>
> Ignore this.
>
>>
>>
>>>>> +static const struct msm_dp_desc sc7280_dp_descs[] = {
>>>>> +     [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = 
>>>>> true },
>>>>> +     [MSM_DP_CONTROLLER_1] = { .io_start = 0x0aea0000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
>>>>>    };
>>>>>
>>>>>    static const struct msm_dp_config sc7280_dp_cfg = {
>>>>> -     .descs = (const struct msm_dp_desc[]) {
>>>>> -             [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = 
>>>>> true },
>>>>> -             [MSM_DP_CONTROLLER_1] = { .io_start = 0x0aea0000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
>>>>> -     },
>>>>> -     .num_descs = 2,
>>>>> +     .descs = sc7280_dp_descs,
>>>>> +     .num_descs = ARRAY_SIZE(sc7280_dp_descs),
>>>>> +};
>>>>> +
>>>>> +static const struct msm_dp_desc sc8180x_dp_descs[] = {
>>>>> +     [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> +     [MSM_DP_CONTROLLER_1] = { .io_start = 0x0ae98000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> +     [MSM_DP_CONTROLLER_2] = { .io_start = 0x0ae9a000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_eDP },
>>>>>    };
>>>>>
>>>>>    static const struct msm_dp_config sc8180x_dp_cfg = {
>>>>> -     .descs = (const struct msm_dp_desc[]) {
>>>>> -             [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> -             [MSM_DP_CONTROLLER_1] = { .io_start = 0x0ae98000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> -             [MSM_DP_CONTROLLER_2] = { .io_start = 0x0ae9a000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_eDP },
>>>>> -     },
>>>>> -     .num_descs = 3,
>>>>> +     .descs = sc8180x_dp_descs,
>>>>> +     .num_descs = ARRAY_SIZE(sc8180x_dp_descs),
>>>>> +};
>>>>> +
>>>>> +static const struct msm_dp_desc sm8350_dp_descs[] = {
>>>>> +     [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>>    };
>>>>>
>>>>>    static const struct msm_dp_config sm8350_dp_cfg = {
>>>>> -     .descs = (const struct msm_dp_desc[]) {
>>>>> -             [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000, 
>>>>> .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
>>>>> -     },
>>>>> -     .num_descs = 1,
>>>>> +     .descs = sm8350_dp_descs,
>>>>> +     .num_descs = ARRAY_SIZE(sm8350_dp_descs),
>>>>>    };
>>>>>
>>>>>    static const struct of_device_id dp_dt_match[] = {
>>>
>>>
>
>