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

[Kuogee Hsieh]

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

At above example table, it just waste one entry. is it ok?

can you elaborate  more on 'look for the DP controller number N' 
functions, where is it?


>>> +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[] = {
>
>