On Mon, 27 Jun 2022 at 22:26, Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > ---
> > 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_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[] = {
--
With best wishes
Dmitry
