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@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_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[] = {
--
With best wishes
Dmitry
