Re: [PATCH v5 7/7] drm/msm/dp: Add sc8180x DP controllers
Quoting Bjorn Andersson (2021-10-16 15:18:43) > The sc8180x has 2 DP and 1 eDP controllers, add support for these to the > DP driver. > > Link: > https://lore.kernel.org/linux-arm-msm/20210725042436.3967173-7-bjorn.anders...@linaro.org/ BTW, was the link intentional?
Re: [PATCH v5 7/7] drm/msm/dp: Add sc8180x DP controllers
Quoting Bjorn Andersson (2021-10-16 15:18:43) > The sc8180x has 2 DP and 1 eDP controllers, add support for these to the > DP driver. > > Link: > https://lore.kernel.org/linux-arm-msm/20210725042436.3967173-7-bjorn.anders...@linaro.org/ > Signed-off-by: Bjorn Andersson > --- Reviewed-by: Stephen Boyd
Re: [PATCH v5 5/7] drm/msm/dp: Support up to 3 DP controllers
Quoting Bjorn Andersson (2021-10-16 15:18:41) > Based on the removal of the g_dp_display and the movement of the > priv->dp lookup into the DP code it's now possible to have multiple > DP instances. > > In line with the other controllers in the MSM driver, introduce a > per-compatible list of base addresses which is used to resolve the > "instance id" for the given DP controller. This instance id is used as > index in the priv->dp[] array. > > Then extend the initialization code to initialize struct drm_encoder for > each of the registered priv->dp[] and update the logic for associating > each struct msm_dp with the struct dpu_encoder_virt. > > A new enum is introduced to document the connection between the > instances referenced in the dpu_intf_cfg array and the controllers in > the DP driver and sc7180 is updated. > > Lastly, bump the number of struct msm_dp instances carries by priv->dp > to 3, the currently known maximum number of controllers found in a > Qualcomm SoC. > > Signed-off-by: Bjorn Andersson > --- Reviewed-by: Stephen Boyd
Re: [PATCH v5 4/7] drm/msm/dp: Allow attaching a drm_panel
Quoting Bjorn Andersson (2021-10-16 15:18:40) > eDP panels might need some power sequencing and backlight management, > so make it possible to associate a drm_panel with an eDP instance and > prepare and enable the panel accordingly. > > Now that we know which hardware instance is DP and which is eDP, > parser->parse() is passed the connector_type and the parser is limited > to only search for a panel in the eDP case. > > Signed-off-by: Bjorn Andersson > --- Reviewed-by: Stephen Boyd
Re: [PATCH v5 3/7] drm/msm/dp: Allow specifying connector_type per controller
Quoting Bjorn Andersson (2021-10-16 15:18:39) > As the following patches introduced support for multiple DP blocks in a > platform and some of those block might be eDP it becomes useful to be > able to specify the connector type per block. > > Although there's only a single block at this point, the array of descs > and the search in dp_display_get_desc() are introduced here to simplify > the next patch, that does introduce support for multiple DP blocks. > > Signed-off-by: Bjorn Andersson > --- Reviewed-by: Stephen Boyd
[PATCH v5 2/2] drm/bridge: lvds-codec: Add support for pixel data sampling edge select
The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to
select input pixel data sampling edge. Add DT property "pclk-sample", not
the same as the one used by display timings but rather the same as used by
media, and configure bus flags based on this DT property.
Signed-off-by: Marek Vasut
Cc: Laurent Pinchart
Cc: Rob Herring
Cc: Sam Ravnborg
Cc: devicet...@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
V2: - Limit the pixelclk-active to encoders only
- Update DT binding document
V3: - Determine whether this is encoder from connector, i.e.
lvds_codec->connector_type == DRM_MODE_CONNECTOR_LVDS
V4: - Switch to pclk-sample. Note that the value of this is inverted,
so all the existing users of pixelclk-active using previous
previous version of this patch must be reworked
V5: Rebase on recent linux-next
---
drivers/gpu/drm/bridge/lvds-codec.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
b/drivers/gpu/drm/bridge/lvds-codec.c
index f991842a161f..702ea803a743 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -21,6 +21,7 @@ struct lvds_codec {
struct device *dev;
struct drm_bridge bridge;
struct drm_bridge *panel_bridge;
+ struct drm_bridge_timings timings;
struct regulator *vcc;
struct gpio_desc *powerdown_gpio;
u32 connector_type;
@@ -119,6 +120,7 @@ static int lvds_codec_probe(struct platform_device *pdev)
struct device_node *bus_node;
struct drm_panel *panel;
struct lvds_codec *lvds_codec;
+ u32 val;
int ret;
lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
@@ -187,12 +189,25 @@ static int lvds_codec_probe(struct platform_device *pdev)
}
}
+ /*
+* Encoder might sample data on different clock edge than the display,
+* for example OnSemi FIN3385 has a dedicated strapping pin to select
+* the sampling edge.
+*/
+ if (lvds_codec->connector_type == DRM_MODE_CONNECTOR_LVDS &&
+ !of_property_read_u32(dev->of_node, "pclk-sample", )) {
+ lvds_codec->timings.input_bus_flags = val ?
+ DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE :
+ DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
+ }
+
/*
* The panel_bridge bridge is attached to the panel's of_node,
* but we need a bridge attached to our of_node for our user
* to look up.
*/
lvds_codec->bridge.of_node = dev->of_node;
+ lvds_codec->bridge.timings = _codec->timings;
drm_bridge_add(_codec->bridge);
platform_set_drvdata(pdev, lvds_codec);
--
2.33.0
[PATCH v5 1/2] dt-bindings: display: bridge: lvds-codec: Document pixel data sampling edge select
The OnSemi FIN3385 Parallel-to-LVDS encoder has a dedicated input line to select input pixel data sampling edge. Add DT property "pclk-sample", not the same as the one used by display timings but rather the same as used by media, to define the pixel data sampling edge. Signed-off-by: Marek Vasut Cc: Laurent Pinchart Cc: Rob Herring Cc: Sam Ravnborg Cc: devicet...@vger.kernel.org To: dri-devel@lists.freedesktop.org --- V4: New patch split from combined V3 V5: Rebase on recent linux-next --- .../bindings/display/bridge/lvds-codec.yaml| 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml index 1faae3e323a4..708de84ac138 100644 --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml @@ -79,6 +79,14 @@ properties: - port@0 - port@1 + pclk-sample: +description: + Data sampling on rising or falling edge. +enum: + - 0 # Falling edge + - 1 # Rising edge +default: 0 + powerdown-gpios: description: The GPIO used to control the power down line of this device. @@ -102,6 +110,16 @@ then: properties: data-mapping: false +if: + not: +properties: + compatible: +contains: + const: lvds-encoder +then: + properties: +pclk-sample: false + required: - compatible - ports -- 2.33.0
[PATCH v5 6/7] dt-bindings: msm/dp: Add SC8180x compatibles
The Qualcomm SC8180x has 2 DP controllers and 1 eDP controller, add compatibles for these to the msm/dp binding. Reviewed-by: Abhinav Kumar Reviewed-by: Stephen Boyd Signed-off-by: Bjorn Andersson --- Changes since v4: - None .../devicetree/bindings/display/msm/dp-controller.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml index 6bb424c21340..63e585f48789 100644 --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml @@ -17,6 +17,8 @@ properties: compatible: enum: - qcom,sc7180-dp + - qcom,sc8180x-dp + - qcom,sc8180x-edp reg: items: -- 2.29.2
[PATCH v5 4/7] drm/msm/dp: Allow attaching a drm_panel
eDP panels might need some power sequencing and backlight management,
so make it possible to associate a drm_panel with an eDP instance and
prepare and enable the panel accordingly.
Now that we know which hardware instance is DP and which is eDP,
parser->parse() is passed the connector_type and the parser is limited
to only search for a panel in the eDP case.
Signed-off-by: Bjorn Andersson
---
Changes since v4:
- None
drivers/gpu/drm/msm/dp/dp_display.c | 9 ++---
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 11 +++
drivers/gpu/drm/msm/dp/dp_parser.c | 30 -
drivers/gpu/drm/msm/dp/dp_parser.h | 3 ++-
5 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 6913970c8cf9..c663cd619925 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include "msm_drv.h"
#include "msm_kms.h"
@@ -230,12 +231,14 @@ static int dp_display_bind(struct device *dev, struct
device *master,
priv = drm->dev_private;
priv->dp = &(dp->dp_display);
- rc = dp->parser->parse(dp->parser);
+ rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
if (rc) {
DRM_ERROR("device tree parsing failed\n");
goto end;
}
+ dp->dp_display.panel_bridge = dp->parser->panel_bridge;
+
dp->aux->drm_dev = drm;
rc = dp_aux_register(dp->aux);
if (rc) {
@@ -822,7 +825,7 @@ static int dp_display_set_mode(struct msm_dp *dp_display,
return 0;
}
-static int dp_display_prepare(struct msm_dp *dp)
+static int dp_display_prepare(struct msm_dp *dp_display)
{
return 0;
}
@@ -896,7 +899,7 @@ static int dp_display_disable(struct dp_display_private
*dp, u32 data)
return 0;
}
-static int dp_display_unprepare(struct msm_dp *dp)
+static int dp_display_unprepare(struct msm_dp *dp_display)
{
return 0;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 75fcabcfbbdd..8e80e3bac394 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -15,6 +15,7 @@ struct msm_dp {
struct device *codec_dev;
struct drm_connector *connector;
struct drm_encoder *encoder;
+ struct drm_bridge *panel_bridge;
bool is_connected;
bool audio_enabled;
bool power_on;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index f33e31523f56..76856c4ee1d6 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include "msm_drv.h"
@@ -160,5 +161,15 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp
*dp_display)
drm_connector_attach_encoder(connector, dp_display->encoder);
+ if (dp_display->panel_bridge) {
+ ret = drm_bridge_attach(dp_display->encoder,
+ dp_display->panel_bridge, NULL,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ if (ret < 0) {
+ DRM_ERROR("failed to attach panel bridge: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+ }
+
return connector;
}
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.c
b/drivers/gpu/drm/msm/dp/dp_parser.c
index 4d6e047f803d..eb6bbfbea484 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.c
+++ b/drivers/gpu/drm/msm/dp/dp_parser.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include "dp_parser.h"
@@ -263,7 +264,28 @@ static int dp_parser_clock(struct dp_parser *parser)
return 0;
}
-static int dp_parser_parse(struct dp_parser *parser)
+static int dp_parser_find_panel(struct dp_parser *parser)
+{
+ struct device *dev = >pdev->dev;
+ struct drm_panel *panel;
+ int rc;
+
+ rc = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, , NULL);
+ if (rc) {
+ DRM_ERROR("failed to acquire DRM panel: %d\n", rc);
+ return rc;
+ }
+
+ parser->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+ if (IS_ERR(parser->panel_bridge)) {
+ DRM_ERROR("failed to create panel bridge\n");
+ return PTR_ERR(parser->panel_bridge);
+ }
+
+ return 0;
+}
+
+static int dp_parser_parse(struct dp_parser *parser, int connector_type)
{
int rc = 0;
@@ -284,6 +306,12 @@ static int dp_parser_parse(struct dp_parser *parser)
if (rc)
return rc;
+ if (connector_type == DRM_MODE_CONNECTOR_eDP) {
+ rc = dp_parser_find_panel(parser);
+ if (rc)
+ return rc;
+ }
+
/* Map the corresponding regulator
[PATCH v5 5/7] drm/msm/dp: Support up to 3 DP controllers
Based on the removal of the g_dp_display and the movement of the
priv->dp lookup into the DP code it's now possible to have multiple
DP instances.
In line with the other controllers in the MSM driver, introduce a
per-compatible list of base addresses which is used to resolve the
"instance id" for the given DP controller. This instance id is used as
index in the priv->dp[] array.
Then extend the initialization code to initialize struct drm_encoder for
each of the registered priv->dp[] and update the logic for associating
each struct msm_dp with the struct dpu_encoder_virt.
A new enum is introduced to document the connection between the
instances referenced in the dpu_intf_cfg array and the controllers in
the DP driver and sc7180 is updated.
Lastly, bump the number of struct msm_dp instances carries by priv->dp
to 3, the currently known maximum number of controllers found in a
Qualcomm SoC.
Signed-off-by: Bjorn Andersson
---
Changes since v4:
- MSM_DP_CONTROLLER_n introduced to clarify the relationship between the intf
specification and the indices of the msm_dp_desc
- Unnecessary parenthesis around dp->dp_display was dropped
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 +++
.../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 8 ++-
drivers/gpu/drm/msm/dp/dp_display.c | 20 --
drivers/gpu/drm/msm/msm_drv.h | 9 ++-
6 files changed, 68 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index b7f33da2799c..9cd9539a1504 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2173,7 +2173,7 @@ int dpu_encoder_setup(struct drm_device *dev, struct
drm_encoder *enc,
dpu_encoder_vsync_event_handler,
0);
else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
- dpu_enc->dp = priv->dp;
+ dpu_enc->dp = priv->dp[disp_info->h_tile_instance[0]];
INIT_DELAYED_WORK(_enc->delayed_off_work,
dpu_encoder_off_work);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 60eed3128b54..47d5d71eb5d3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -900,7 +900,7 @@ static const struct dpu_intf_cfg sdm845_intf[] = {
};
static const struct dpu_intf_cfg sc7180_intf[] = {
- INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC7180_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC7180_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index f655adbc2421..875b07e7183d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -188,6 +188,7 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct
drm_minor *minor)
struct dentry *entry;
struct drm_device *dev;
struct msm_drm_private *priv;
+ int i;
if (!p)
return -EINVAL;
@@ -203,8 +204,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms,
struct drm_minor *minor)
dpu_debugfs_vbif_init(dpu_kms, entry);
dpu_debugfs_core_irq_init(dpu_kms, entry);
- if (priv->dp)
- msm_dp_debugfs_init(priv->dp, minor);
+ for (i = 0; i < ARRAY_SIZE(priv->dp); i++) {
+ if (priv->dp[i])
+ msm_dp_debugfs_init(priv->dp[i], minor);
+ }
return dpu_core_perf_debugfs_init(dpu_kms, entry);
}
@@ -544,35 +547,42 @@ static int _dpu_kms_initialize_displayport(struct
drm_device *dev,
{
struct drm_encoder *encoder = NULL;
struct msm_display_info info;
- int rc = 0;
+ int rc;
+ int i;
- if (!priv->dp)
- return rc;
+ for (i = 0; i < ARRAY_SIZE(priv->dp); i++) {
+ if (!priv->dp[i])
+ continue;
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
- if (IS_ERR(encoder)) {
- DPU_ERROR("encoder init failed for dsi display\n");
- return PTR_ERR(encoder);
- }
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+ if (IS_ERR(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return PTR_ERR(encoder);
+ }
- memset(, 0, sizeof(info));
- rc = msm_dp_modeset_init(priv->dp, dev, encoder);
- if (rc) {
-
[PATCH v5 7/7] drm/msm/dp: Add sc8180x DP controllers
The sc8180x has 2 DP and 1 eDP controllers, add support for these to the
DP driver.
Link:
https://lore.kernel.org/linux-arm-msm/20210725042436.3967173-7-bjorn.anders...@linaro.org/
Signed-off-by: Bjorn Andersson
---
Changes since v4:
- Use the MSM_DP_CONTROLLER_n enums
- const the msm_dp_desc array
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 6 +++---
drivers/gpu/drm/msm/dp/dp_display.c| 11 +++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 47d5d71eb5d3..0ac6a79e8af9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -918,13 +918,13 @@ static const struct dpu_intf_cfg sc7280_intf[] = {
};
static const struct dpu_intf_cfg sc8180x_intf[] = {
- INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, 0, 24, INTF_SC8180X_MASK,
MDP_SSPP_TOP0_INTR, 24, 25),
+ INTF_BLK("intf_0", INTF_0, 0x6A000, INTF_DP, MSM_DP_CONTROLLER_0, 24,
INTF_SC8180X_MASK, MDP_SSPP_TOP0_INTR, 24, 25),
INTF_BLK("intf_1", INTF_1, 0x6A800, INTF_DSI, 0, 24, INTF_SC8180X_MASK,
MDP_SSPP_TOP0_INTR, 26, 27),
INTF_BLK("intf_2", INTF_2, 0x6B000, INTF_DSI, 1, 24, INTF_SC8180X_MASK,
MDP_SSPP_TOP0_INTR, 28, 29),
/* INTF_3 is for MST, wired to INTF_DP 0 and 1, use dummy index until
this is supported */
INTF_BLK("intf_3", INTF_3, 0x6B800, INTF_DP, 999, 24,
INTF_SC8180X_MASK, MDP_SSPP_TOP0_INTR, 30, 31),
- INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, 1, 24, INTF_SC8180X_MASK,
MDP_SSPP_TOP0_INTR, 20, 21),
- INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, 2, 24, INTF_SC8180X_MASK,
MDP_SSPP_TOP0_INTR, 22, 23),
+ INTF_BLK("intf_4", INTF_4, 0x6C000, INTF_DP, MSM_DP_CONTROLLER_1, 24,
INTF_SC8180X_MASK, MDP_SSPP_TOP0_INTR, 20, 21),
+ INTF_BLK("intf_5", INTF_5, 0x6C800, INTF_DP, MSM_DP_CONTROLLER_2, 24,
INTF_SC8180X_MASK, MDP_SSPP_TOP0_INTR, 22, 23),
};
/*
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index d3c9d7273354..70dcd4e6d466 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -135,8 +135,19 @@ static const struct msm_dp_config sc7180_dp_cfg = {
.num_descs = 1,
};
+static const struct msm_dp_config sc8180x_dp_cfg = {
+ .descs = (const struct msm_dp_desc[]) {
+ [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
.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,
+};
+
static const struct of_device_id dp_dt_match[] = {
{ .compatible = "qcom,sc7180-dp", .data = _dp_cfg },
+ { .compatible = "qcom,sc8180x-dp", .data = _dp_cfg },
+ { .compatible = "qcom,sc8180x-edp", .data = _dp_cfg },
{}
};
--
2.29.2
[PATCH v5 3/7] drm/msm/dp: Allow specifying connector_type per controller
As the following patches introduced support for multiple DP blocks in a
platform and some of those block might be eDP it becomes useful to be
able to specify the connector type per block.
Although there's only a single block at this point, the array of descs
and the search in dp_display_get_desc() are introduced here to simplify
the next patch, that does introduce support for multiple DP blocks.
Signed-off-by: Bjorn Andersson
---
Changes since v4:
- const the various struct msm_dp_desc instances
- unsigned the connector_type
The references to DRM_MODE_CONNECTOR_DisplayPort in dp_debug.c, that was
highligted in the review of v4 has been removed in a separate patch.
drivers/gpu/drm/msm/dp/dp_display.c | 43 -
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 2 +-
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 5d3ee5ef07c2..6913970c8cf9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -115,8 +115,25 @@ struct dp_display_private {
struct dp_audio *audio;
};
+struct msm_dp_desc {
+ phys_addr_t io_start;
+ unsigned int connector_type;
+};
+
+struct msm_dp_config {
+ const struct msm_dp_desc *descs;
+ size_t num_descs;
+};
+
+static const struct msm_dp_config sc7180_dp_cfg = {
+ .descs = (const struct msm_dp_desc[]) {
+ { .io_start = 0x0ae9, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
+ },
+ .num_descs = 1,
+};
+
static const struct of_device_id dp_dt_match[] = {
- {.compatible = "qcom,sc7180-dp"},
+ { .compatible = "qcom,sc7180-dp", .data = _dp_cfg },
{}
};
@@ -1180,10 +1197,29 @@ int dp_display_request_irq(struct msm_dp *dp_display)
return 0;
}
+static const struct msm_dp_desc *dp_display_get_desc(struct platform_device
*pdev)
+{
+ const struct msm_dp_config *cfg = of_device_get_match_data(>dev);
+ struct resource *res;
+ int i;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return NULL;
+
+ for (i = 0; i < cfg->num_descs; i++)
+ if (cfg->descs[i].io_start == res->start)
+ return >descs[i];
+
+ dev_err(>dev, "unknown displayport instance\n");
+ return NULL;
+}
+
static int dp_display_probe(struct platform_device *pdev)
{
int rc = 0;
struct dp_display_private *dp;
+ const struct msm_dp_desc *desc;
if (!pdev || !pdev->dev.of_node) {
DRM_ERROR("pdev not found\n");
@@ -1194,8 +1230,13 @@ static int dp_display_probe(struct platform_device *pdev)
if (!dp)
return -ENOMEM;
+ desc = dp_display_get_desc(pdev);
+ if (!desc)
+ return -EINVAL;
+
dp->pdev = pdev;
dp->name = "drm_dp";
+ dp->dp_display.connector_type = desc->connector_type;
rc = dp_init_sub_modules(dp);
if (rc) {
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h
b/drivers/gpu/drm/msm/dp/dp_display.h
index 8b47cdabb67e..75fcabcfbbdd 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -18,6 +18,7 @@ struct msm_dp {
bool is_connected;
bool audio_enabled;
bool power_on;
+ unsigned int connector_type;
hdmi_codec_plugged_cb plugged_cb;
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 764f4b81017e..f33e31523f56 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -147,7 +147,7 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp
*dp_display)
ret = drm_connector_init(dp_display->drm_dev, connector,
_connector_funcs,
- DRM_MODE_CONNECTOR_DisplayPort);
+ dp_display->connector_type);
if (ret)
return ERR_PTR(ret);
--
2.29.2
[PATCH v5 2/7] drm/msm/dp: Modify prototype of encoder based API
Functions in the DisplayPort code that relates to individual instances
(encoders) are passed both the struct msm_dp and the struct drm_encoder.
But in a situation where multiple DP instances would exist this means
that the caller need to resolve which struct msm_dp relates to the
struct drm_encoder at hand.
Store a reference to the struct msm_dp associated with each
dpu_encoder_virt to allow the particular instance to be associate with
the encoder in the following patch.
Reviewed-by: Abhinav Kumar
Reviewed-by: Stephen Boyd
Signed-off-by: Bjorn Andersson
---
Changes since v4:
- None
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 -
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0e9d3fa1544b..b7f33da2799c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -168,6 +168,7 @@ enum dpu_enc_rc_states {
* @vsync_event_work: worker to handle vsync event for autorefresh
* @topology: topology of the display
* @idle_timeout: idle timeout duration in milliseconds
+ * @dp:msm_dp pointer, for DP encoders
*/
struct dpu_encoder_virt {
struct drm_encoder base;
@@ -206,6 +207,8 @@ struct dpu_encoder_virt {
struct msm_display_topology topology;
u32 idle_timeout;
+
+ struct msm_dp *dp;
};
#define to_dpu_encoder_virt(x) container_of(x, struct dpu_encoder_virt, base)
@@ -1000,8 +1003,8 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder
*drm_enc,
trace_dpu_enc_mode_set(DRMID(drm_enc));
- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp)
- msm_dp_display_mode_set(priv->dp, drm_enc, mode, adj_mode);
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS)
+ msm_dp_display_mode_set(dpu_enc->dp, drm_enc, mode, adj_mode);
list_for_each_entry(conn_iter, connector_list, head)
if (conn_iter->encoder == drm_enc)
@@ -1182,9 +1185,8 @@ static void dpu_encoder_virt_enable(struct drm_encoder
*drm_enc)
_dpu_encoder_virt_enable_helper(drm_enc);
- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) {
- ret = msm_dp_display_enable(priv->dp,
- drm_enc);
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) {
+ ret = msm_dp_display_enable(dpu_enc->dp, drm_enc);
if (ret) {
DPU_ERROR_ENC(dpu_enc, "dp display enable failed: %d\n",
ret);
@@ -1224,8 +1226,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder
*drm_enc)
/* wait for idle */
dpu_encoder_wait_for_event(drm_enc, MSM_ENC_TX_COMPLETE);
- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) {
- if (msm_dp_display_pre_disable(priv->dp, drm_enc))
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) {
+ if (msm_dp_display_pre_disable(dpu_enc->dp, drm_enc))
DPU_ERROR_ENC(dpu_enc, "dp display push idle failed\n");
}
@@ -1253,8 +1255,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder
*drm_enc)
DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
- if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS && priv->dp) {
- if (msm_dp_display_disable(priv->dp, drm_enc))
+ if (drm_enc->encoder_type == DRM_MODE_ENCODER_TMDS) {
+ if (msm_dp_display_disable(dpu_enc->dp, drm_enc))
DPU_ERROR_ENC(dpu_enc, "dp display disable failed\n");
}
@@ -2170,7 +2172,8 @@ int dpu_encoder_setup(struct drm_device *dev, struct
drm_encoder *enc,
timer_setup(_enc->vsync_event_timer,
dpu_encoder_vsync_event_handler,
0);
-
+ else if (disp_info->intf_type == DRM_MODE_ENCODER_TMDS)
+ dpu_enc->dp = priv->dp;
INIT_DELAYED_WORK(_enc->delayed_off_work,
dpu_encoder_off_work);
--
2.29.2
[PATCH v5 1/7] drm/msm/dp: Remove global g_dp_display variable
As the Qualcomm DisplayPort driver only supports a single instance of
the driver the commonly used struct dp_display is kept in a global
variable. As we introduce additional instances this obviously doesn't
work.
Replace this with a combination of existing references to adjacent
objects and drvdata.
Reviewed-by: Abhinav Kumar
Reviewed-by: Stephen Boyd
Signed-off-by: Bjorn Andersson
---
Changes since v4:
- None
drivers/gpu/drm/msm/dp/dp_display.c | 80 -
1 file changed, 21 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index fbe4c2cd52a3..5d3ee5ef07c2 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -27,7 +27,6 @@
#include "dp_audio.h"
#include "dp_debug.h"
-static struct msm_dp *g_dp_display;
#define HPD_STRING_SIZE 30
enum {
@@ -121,6 +120,13 @@ static const struct of_device_id dp_dt_match[] = {
{}
};
+static struct dp_display_private *dev_get_dp_display_private(struct device
*dev)
+{
+ struct msm_dp *dp = dev_get_drvdata(dev);
+
+ return container_of(dp, struct dp_display_private, dp_display);
+}
+
static int dp_add_event(struct dp_display_private *dp_priv, u32 event,
u32 data, u32 delay)
{
@@ -197,15 +203,12 @@ static int dp_display_bind(struct device *dev, struct
device *master,
void *data)
{
int rc = 0;
- struct dp_display_private *dp;
- struct drm_device *drm;
+ struct dp_display_private *dp = dev_get_dp_display_private(dev);
struct msm_drm_private *priv;
+ struct drm_device *drm;
drm = dev_get_drvdata(master);
- dp = container_of(g_dp_display,
- struct dp_display_private, dp_display);
-
dp->dp_display.drm_dev = drm;
priv = drm->dev_private;
priv->dp = &(dp->dp_display);
@@ -240,13 +243,10 @@ static int dp_display_bind(struct device *dev, struct
device *master,
static void dp_display_unbind(struct device *dev, struct device *master,
void *data)
{
- struct dp_display_private *dp;
+ struct dp_display_private *dp = dev_get_dp_display_private(dev);
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
- dp = container_of(g_dp_display,
- struct dp_display_private, dp_display);
-
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp = NULL;
@@ -379,38 +379,17 @@ static void dp_display_host_deinit(struct
dp_display_private *dp)
static int dp_display_usbpd_configure_cb(struct device *dev)
{
- int rc = 0;
- struct dp_display_private *dp;
-
- if (!dev) {
- DRM_ERROR("invalid dev\n");
- rc = -EINVAL;
- goto end;
- }
-
- dp = container_of(g_dp_display,
- struct dp_display_private, dp_display);
+ struct dp_display_private *dp = dev_get_dp_display_private(dev);
dp_display_host_init(dp, false);
- rc = dp_display_process_hpd_high(dp);
-end:
- return rc;
+ return dp_display_process_hpd_high(dp);
}
static int dp_display_usbpd_disconnect_cb(struct device *dev)
{
int rc = 0;
- struct dp_display_private *dp;
-
- if (!dev) {
- DRM_ERROR("invalid dev\n");
- rc = -EINVAL;
- return rc;
- }
-
- dp = container_of(g_dp_display,
- struct dp_display_private, dp_display);
+ struct dp_display_private *dp = dev_get_dp_display_private(dev);
dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
@@ -472,15 +451,7 @@ static int dp_display_usbpd_attention_cb(struct device
*dev)
{
int rc = 0;
u32 sink_request;
- struct dp_display_private *dp;
-
- if (!dev) {
- DRM_ERROR("invalid dev\n");
- return -EINVAL;
- }
-
- dp = container_of(g_dp_display,
- struct dp_display_private, dp_display);
+ struct dp_display_private *dp = dev_get_dp_display_private(dev);
/* check for any test request issued by sink */
rc = dp_link_process_request(dp->link);
@@ -647,7 +618,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private
*dp, u32 data)
DRM_DEBUG_DP("hpd_state=%d\n", state);
/* signal the disconnect event early to ensure proper teardown */
- dp_display_handle_plugged_change(g_dp_display, false);
+ dp_display_handle_plugged_change(>dp_display, false);
/* enable HDP plug interrupt to prepare for next plugin */
dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true);
@@ -842,9 +813,7 @@ static int dp_display_prepare(struct msm_dp *dp)
static int dp_display_enable(struct dp_display_private
[PATCH v5 0/5] drm/msm/dp: Support multiple DP instances and add sc8180x
The current implementation supports a single DP instance and the DPU code will only match it against INTF_DP instance 0. These patches extends this to allow multiple DP instances and support for matching against DP instances beyond 0. With that in place add SC8180x DP and eDP controllers. Bjorn Andersson (7): drm/msm/dp: Remove global g_dp_display variable drm/msm/dp: Modify prototype of encoder based API drm/msm/dp: Allow specifying connector_type per controller drm/msm/dp: Allow attaching a drm_panel drm/msm/dp: Support up to 3 DP controllers dt-bindings: msm/dp: Add SC8180x compatibles drm/msm/dp: Add sc8180x DP controllers .../bindings/display/msm/dp-controller.yaml | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 23 +-- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 .../gpu/drm/msm/disp/msm_disp_snapshot_util.c | 8 +- drivers/gpu/drm/msm/dp/dp_display.c | 153 ++ drivers/gpu/drm/msm/dp/dp_display.h | 2 + drivers/gpu/drm/msm/dp/dp_drm.c | 13 +- drivers/gpu/drm/msm/dp/dp_parser.c| 30 +++- drivers/gpu/drm/msm/dp/dp_parser.h| 3 +- drivers/gpu/drm/msm/msm_drv.h | 9 +- 11 files changed, 205 insertions(+), 112 deletions(-) -- 2.29.2
Re: [PATCH] drm/bridge: ti-sn65dsi83: Optimize reset line toggling
On Sat, Oct 16, 2021 at 11:04 PM Marek Vasut wrote: > Current code always sets reset line low in .pre_enable callback and > holds it low for 10ms. This is sub-optimal and increases the time > between enablement of the DSI83 and valid LVDS clock. > > Rework the reset handling such that the reset line is held low for 10ms > both in probe() of the driver and .disable callback, which guarantees > that the reset line was always held low for more than 10ms and therefore > the reset line timing requirement is satisfied. Furthermore, move the > reset handling into .enable callback so the entire DSI83 initialization > is now in one place. > > This reduces DSI83 enablement delay by up to 10ms. > > Signed-off-by: Marek Vasut > Cc: Laurent Pinchart > Cc: Linus Walleij > Cc: Robert Foss > Cc: Sam Ravnborg > Cc: dri-devel@lists.freedesktop.org This looks good to me. Reviewed-by: Linus Walleij Yours, Linus Walleij
[PATCH] drm: mxsfb: Fix NULL pointer dereference crash on unload
The mxsfb->crtc.funcs may already be NULL when unloading the driver,
in which case calling mxsfb_irq_disable() via drm_irq_uninstall() from
mxsfb_unload() leads to NULL pointer dereference.
Since all we care about is masking the IRQ and mxsfb->base is still
valid, just use that to clear and mask the IRQ.
Fixes: ae1ed00932819 ("drm: mxsfb: Stop using DRM simple display pipeline
helper")
Signed-off-by: Marek Vasut
Cc: Daniel Abrecht
Cc: Emil Velikov
Cc: Laurent Pinchart
Cc: Sam Ravnborg
Cc: Stefan Agner
---
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index ec0432fe1bdf..86d78634a979 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -173,7 +173,11 @@ static void mxsfb_irq_disable(struct drm_device *drm)
struct mxsfb_drm_private *mxsfb = drm->dev_private;
mxsfb_enable_axi_clk(mxsfb);
- mxsfb->crtc.funcs->disable_vblank(>crtc);
+
+ /* Disable and clear VBLANK IRQ */
+ writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR);
+ writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR);
+
mxsfb_disable_axi_clk(mxsfb);
}
--
2.33.0
[PATCH] drm/bridge: ti-sn65dsi83: Optimize reset line toggling
Current code always sets reset line low in .pre_enable callback and
holds it low for 10ms. This is sub-optimal and increases the time
between enablement of the DSI83 and valid LVDS clock.
Rework the reset handling such that the reset line is held low for 10ms
both in probe() of the driver and .disable callback, which guarantees
that the reset line was always held low for more than 10ms and therefore
the reset line timing requirement is satisfied. Furthermore, move the
reset handling into .enable callback so the entire DSI83 initialization
is now in one place.
This reduces DSI83 enablement delay by up to 10ms.
Signed-off-by: Marek Vasut
Cc: Laurent Pinchart
Cc: Linus Walleij
Cc: Robert Foss
Cc: Sam Ravnborg
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 40 ---
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index ba1160ec6d6e..52030a82f3e1 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -301,22 +301,6 @@ static void sn65dsi83_detach(struct drm_bridge *bridge)
ctx->dsi = NULL;
}
-static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge,
- struct drm_bridge_state
*old_bridge_state)
-{
- struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
-
- /*
-* Reset the chip, pull EN line low for t_reset=10ms,
-* then high for t_en=1ms.
-*/
- regcache_mark_dirty(ctx->regmap);
- gpiod_set_value(ctx->enable_gpio, 0);
- usleep_range(1, 11000);
- gpiod_set_value(ctx->enable_gpio, 1);
- usleep_range(1000, 1100);
-}
-
static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx,
const struct drm_display_mode *mode)
{
@@ -394,6 +378,10 @@ static void sn65dsi83_atomic_enable(struct drm_bridge
*bridge,
u16 val;
int ret;
+ /* Deassert reset */
+ gpiod_set_value(ctx->enable_gpio, 1);
+ usleep_range(1000, 1100);
+
/* Get the LVDS format from the bridge state. */
bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
@@ -540,18 +528,11 @@ static void sn65dsi83_atomic_disable(struct drm_bridge
*bridge,
{
struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
- /* Clear reset, disable PLL */
- regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
- regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
-}
-
-static void sn65dsi83_atomic_post_disable(struct drm_bridge *bridge,
- struct drm_bridge_state
*old_bridge_state)
-{
- struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
-
- /* Put the chip in reset, pull EN line low. */
+ /* Put the chip in reset, pull EN line low, and assure 10ms reset low
timing. */
gpiod_set_value(ctx->enable_gpio, 0);
+ usleep_range(1, 11000);
+
+ regcache_mark_dirty(ctx->regmap);
}
static enum drm_mode_status
@@ -597,10 +578,8 @@ sn65dsi83_atomic_get_input_bus_fmts(struct drm_bridge
*bridge,
static const struct drm_bridge_funcs sn65dsi83_funcs = {
.attach = sn65dsi83_attach,
.detach = sn65dsi83_detach,
- .atomic_pre_enable = sn65dsi83_atomic_pre_enable,
.atomic_enable = sn65dsi83_atomic_enable,
.atomic_disable = sn65dsi83_atomic_disable,
- .atomic_post_disable= sn65dsi83_atomic_post_disable,
.mode_valid = sn65dsi83_mode_valid,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
@@ -685,10 +664,13 @@ static int sn65dsi83_probe(struct i2c_client *client,
model = id->driver_data;
}
+ /* Put the chip in reset, pull EN line low, and assure 10ms reset low
timing. */
ctx->enable_gpio = devm_gpiod_get(ctx->dev, "enable", GPIOD_OUT_LOW);
if (IS_ERR(ctx->enable_gpio))
return PTR_ERR(ctx->enable_gpio);
+ usleep_range(1, 11000);
+
ret = sn65dsi83_parse_dt(ctx, model);
if (ret)
return ret;
--
2.33.0
Re: [bug report] drm/msm: dsi: Handle dual-channel for 6G as well
On Fri, Oct 15, 2021 at 12:34:20PM -0700, Jessica Zhang wrote:
> Hey Dmitry,
>
> On 10/15/2021 11:24 AM, Dmitry Baryshkov wrote:
> > On Fri, 15 Oct 2021 at 04:43, Jessica Zhang wrote:
> > > Hey Dan,
> > >
> > > On 10/1/2021 5:31 AM, Dan Carpenter wrote:
> > > > Hello Sean Paul,
> > > >
> > > > The patch a6bcddbc2ee1: "drm/msm: dsi: Handle dual-channel for 6G as
> > > > well" from Jul 25, 2018, leads to the following
> > > > Smatch static checker warning:
> > > >
> > > >drivers/gpu/drm/msm/dsi/dsi_host.c:729 dsi_calc_clk_rate_6g()
> > > >warn: wrong type for 'msm_host->esc_clk_rate' (should be 'ulong')
> > > >
> > > > drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > 721 int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool
> > > > is_bonded_dsi)
> > > > 722 {
> > > > 723 if (!msm_host->mode) {
> > > > 724 pr_err("%s: mode not set\n", __func__);
> > > > 725 return -EINVAL;
> > > > 726 }
> > > > 727
> > > > 728 dsi_calc_pclk(msm_host, is_bonded_dsi);
> > > > --> 729 msm_host->esc_clk_rate =
> > > > clk_get_rate(msm_host->esc_clk);
> > > > ^^
> > > > I don't know why Smatch is suddenly warning about ancient msm code, but
> > > > clock rates should be unsigned long. (I don't remember why).
> > > >
> > > > 730 return 0;
> > > > 731 }
> > > I'm unable to recreate the warning with Smatch. After running
> > > build_kernel_data.sh, I ran `/smatch_scripts/kchecker
> > > drivers/gpu/drm/msm/dsi/dsi_host.c` and got the following output:
> > >
> > > CHECK scripts/mod/empty.c
> > > CALL scripts/checksyscalls.sh
> > > CALL scripts/atomic/check-atomics.sh
> > > CHECK arch/arm64/kernel/vdso/vgettimeofday.c
> > > CC drivers/gpu/drm/msm/dsi/dsi_host.o
> > > CHECK drivers/gpu/drm/msm/dsi/dsi_host.c
> > > drivers/gpu/drm/msm/dsi/dsi_host.c:2380 msm_dsi_host_power_on() warn:
> > > missing error code 'ret'
> > >
> > > Is there a specific .config you're using (that's not the default
> > > mainline defconfig)? If so, can you please share it?
> > Are you running your checks with ARM32 or ARM64 in mind?
> > Note, esc_clk_rate is u32, while clk_get_rate()'s returns unsigned long.
> > It would make sense to change all three clocks rates in msm_dsi_host
> > struct (and several places where they are used) to unsigned long.
>
> Thanks for the response. I'm aware of what's causing this issue and how to
> fix it, but I want to also be able to recreate the warning locally with
> Smatch.
No, sorry, I haven't published that check. It's just something I have
locally.
Btw, I will be offline for the next two weeks...
regards,
dan carpenter
[PATCH v2 08/13] drm/sun4i: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 2f2c9f0a1071..f57bedbbeeb8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -215,11 +215,11 @@ static int sun4i_hdmi_get_modes(struct drm_connector
*connector)
if (!edid)
return 0;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+ drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_monitor = connector->display_info.is_hdmi;
DRM_DEBUG_DRIVER("Monitor is %s monitor\n",
hdmi->hdmi_monitor ? "an HDMI" : "a DVI");
- drm_connector_update_edid_property(connector, edid);
cec_s_phys_addr_from_edid(hdmi->cec_adap, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
--
2.33.0
[PATCH v2 12/13] drm/nouveau: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++--
drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +---
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..b3c7199aa63d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -844,7 +844,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct
nouveau_crtc *nv_crtc,
int ret;
int size;
- if (!drm_detect_hdmi_monitor(nv_connector->edid))
+ if (!nv_connector->base.display_info.is_hdmi)
return;
hdmi = _connector->base.display_info.hdmi;
@@ -1745,7 +1745,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder,
struct drm_atomic_state *sta
*/
if (mode->clock >= 165000 &&
nv_encoder->dcb->duallink_possible &&
- !drm_detect_hdmi_monitor(nv_connector->edid))
+ !nv_connector->base.display_info.is_hdmi)
proto =
NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS;
} else {
proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index d66f97280282..29d6c010ac13 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
struct drm_display_mode *omode = >state.adjusted_mode;
struct drm_display_mode *umode = >state.mode;
int mode = asyc->scaler.mode;
- struct edid *edid;
int umode_vdisplay, omode_hdisplay, omode_vdisplay;
- if (connector->edid_blob_ptr)
- edid = (struct edid *)connector->edid_blob_ptr->data;
- else
- edid = NULL;
-
if (!asyc->scaler.full) {
if (mode == DRM_MODE_SCALE_NONE)
omode = umode;
@@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
*/
if ((asyc->scaler.underscan.mode == UNDERSCAN_ON ||
(asyc->scaler.underscan.mode == UNDERSCAN_AUTO &&
-drm_detect_hdmi_monitor(edid {
+connector->display_info.is_hdmi))) {
u32 bX = asyc->scaler.underscan.hborder;
u32 bY = asyc->scaler.underscan.vborder;
u32 r = (asyh->view.oH << 19) / asyh->view.oW;
--
2.33.0
[PATCH v2 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..008e5b0ba408 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2355,7 +2355,7 @@ intel_hdmi_set_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = edid;
if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
- intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
+ intel_hdmi->has_hdmi_sink = connector->display_info.is_hdmi;
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..b4065e4df644 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -2060,8 +2060,9 @@ intel_sdvo_tmds_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (intel_sdvo_connector->is_hdmi) {
- intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ intel_sdvo->has_hdmi_monitor =
+
connector->display_info.is_hdmi;
}
} else
status = connector_status_disconnected;
--
2.33.0
[PATCH v2 05/13] drm/gma500: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++-
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 --
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index e525689f84f0..d9db5d52d52e 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -130,6 +130,7 @@ static enum drm_connector_status cdv_hdmi_detect(
struct edid *edid = NULL;
enum drm_connector_status status = connector_status_disconnected;
+ /* This updates connector->display_info */
edid = drm_get_edid(connector, _encoder->i2c_bus->adapter);
hdmi_priv->has_hdmi_sink = false;
@@ -138,7 +139,7 @@ static enum drm_connector_status cdv_hdmi_detect(
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
hdmi_priv->has_hdmi_sink =
- drm_detect_hdmi_monitor(edid);
+ connector->display_info.is_hdmi;
hdmi_priv->has_hdmi_audio =
drm_detect_monitor_audio(edid);
}
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 355da2856389..5ef49d17de98 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -1266,8 +1266,10 @@ psb_intel_sdvo_hdmi_sink_detect(struct drm_connector
*connector)
if (edid->input & DRM_EDID_INPUT_DIGITAL) {
status = connector_status_connected;
if (psb_intel_sdvo->is_hdmi) {
- psb_intel_sdvo->has_hdmi_monitor =
drm_detect_hdmi_monitor(edid);
- psb_intel_sdvo->has_hdmi_audio =
drm_detect_monitor_audio(edid);
+ psb_intel_sdvo->has_hdmi_monitor =
+ connector->display_info.is_hdmi;
+ psb_intel_sdvo->has_hdmi_audio =
+ drm_detect_monitor_audio(edid);
}
} else
status = connector_status_disconnected;
--
2.33.0
[PATCH v2 09/13] drm/sti: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/sti/sti_hdmi.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f3ace11209dd..3f8b04a1407e 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -984,14 +984,16 @@ static int sti_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
goto fail;
- hdmi->hdmi_monitor = drm_detect_hdmi_monitor(edid);
- DRM_DEBUG_KMS("%s : %dx%d cm\n",
- (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
- edid->width_cm, edid->height_cm);
cec_notifier_set_phys_addr_from_edid(hdmi->notifier, edid);
count = drm_add_edid_modes(connector, edid);
+
+ /* This updates connector->display_info */
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_monitor = connector->display_info.is_hdmi;
+ DRM_DEBUG_KMS("%s : %dx%d cm\n",
+ (hdmi->hdmi_monitor ? "hdmi monitor" : "dvi monitor"),
+ edid->width_cm, edid->height_cm);
kfree(edid);
return count;
--
2.33.0
[PATCH v2 06/13] drm/exynos: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Retriving the same information with drm_detect_hdmi_monitor() is less efficient. Change to drm_display_info.is_hdmi Signed-off-by: Claudio Suarez --- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 7655142a4651..a563d6386abe 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -893,12 +893,14 @@ static int hdmi_get_modes(struct drm_connector *connector) if (!edid) return -ENODEV; - hdata->dvi_mode = !drm_detect_hdmi_monitor(edid); + /* This updates connector->display_info */ + drm_connector_update_edid_property(connector, edid); + + hdata->dvi_mode = !connector->display_info.is_hdmi; DRM_DEV_DEBUG_KMS(hdata->dev, "%s : width[%d] x height[%d]\n", (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"), edid->width_cm, edid->height_cm); - drm_connector_update_edid_property(connector, edid); cec_notifier_set_phys_addr_from_edid(hdata->notifier, edid); ret = drm_add_edid_modes(connector, edid); -- 2.33.0
[PATCH v2 07/13] drm/msm: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 58707a1f3878..07585092f919 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -364,8 +364,8 @@ static int msm_hdmi_connector_get_modes(struct
drm_connector *connector)
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl);
- hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_mode = connector->display_info.is_hdmi;
if (edid) {
ret = drm_add_edid_modes(connector, edid);
--
2.33.0
[PATCH v2 10/13] drm/rockchip: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++--
drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c
b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 7afdc54eb3ec..d479f230833e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -553,9 +553,9 @@ static int inno_hdmi_connector_get_modes(struct
drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
- hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
- hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
+ hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 1c546c3a8998..03aaae39cf61 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -472,8 +472,8 @@ static int rk3066_hdmi_connector_get_modes(struct
drm_connector *connector)
edid = drm_get_edid(connector, hdmi->ddc);
if (edid) {
- hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
drm_connector_update_edid_property(connector, edid);
+ hdmi->hdmi_data.sink_is_hdmi = connector->display_info.is_hdmi;
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
}
--
2.33.0
[PATCH v2 11/13] drm/bridge: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi where possible
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
drivers/gpu/drm/bridge/sii902x.c | 2 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 76555ae64e9c..f6891280a58d 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -617,7 +617,7 @@ static struct edid *adv7511_get_edid(struct adv7511
*adv7511,
__adv7511_power_off(adv7511);
adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
- drm_detect_hdmi_monitor(edid));
+ connector->display_info.is_hdmi);
cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 89558e581530..5719be0a03c7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -283,7 +283,7 @@ static int sii902x_get_modes(struct drm_connector
*connector)
edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
drm_connector_update_edid_property(connector, edid);
if (edid) {
- if (drm_detect_hdmi_monitor(edid))
+ if (connector->display_info.is_hdmi)
output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
num = drm_add_edid_modes(connector, edid);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f08d0fded61f..33f0afb6b646 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2359,7 +2359,7 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi,
dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
edid->width_cm, edid->height_cm);
- hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
+ hdmi->sink_is_hdmi = connector->display_info.is_hdmi;
hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
return edid;
--
2.33.0
[PATCH v2 04/13] drm/tegra: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information with
drm_detect_hdmi_monitor() is less efficient. Change to
drm_display_info.is_hdmi
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/tegra/hdmi.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index e5d2a4026028..21571221b49b 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -831,14 +831,10 @@ static void tegra_hdmi_setup_tmds(struct tegra_hdmi *hdmi,
static bool tegra_output_is_hdmi(struct tegra_output *output)
{
- struct edid *edid;
-
if (!output->connector.edid_blob_ptr)
return false;
- edid = (struct edid *)output->connector.edid_blob_ptr->data;
-
- return drm_detect_hdmi_monitor(edid);
+ return output->connector.display_info.is_hdmi;
}
static enum drm_connector_status
--
2.33.0
Re: [PATCH v2 1/2] drm/panel: Add driver for LG.Philips SW43101 DSI video mode panel
Hi Yassine,
Sorry for the late response - have been away from kernel stuff for some
months.
A few comments in the following - mostly backlight related.
Please fix and resend.
Note: bindings comes before the panel driver - as we cannot apply a
panel driver for an unknown binding.
Sam
On Thu, Sep 09, 2021 at 04:40:02AM +, Yassine Oudjana wrote:
> Add a driver for the LG.Philips SW43101 FHD (1080x1920) OLED DSI video mode
> panel.
> This driver has been generated using linux-mdss-dsi-panel-driver-generator.
>
> Signed-off-by: Yassine Oudjana
> ---
> Changes since v1:
> - Add regulator support.
> - Add MAINTAINERS entry.
>
> MAINTAINERS | 5 +
> drivers/gpu/drm/panel/Kconfig | 10 +
> drivers/gpu/drm/panel/Makefile| 1 +
> .../gpu/drm/panel/panel-lgphilips-sw43101.c | 363 ++
> 4 files changed, 379 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-lgphilips-sw43101.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f58dad1a1922..46431e8ad373 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5899,6 +5899,11 @@ S: Orphan / Obsolete
> F: drivers/gpu/drm/i810/
> F: include/uapi/drm/i810_drm.h
>
> +DRM DRIVER FOR LG.PHILIPS SW43101 PANEL
> +M: Yassine Oudjana
> +S: Maintained
> +F: drivers/gpu/drm/panel/panel-lgphilips-sw43101.c
> +
> DRM DRIVER FOR LVDS PANELS
> M: Laurent Pinchart
> L: dri-devel@lists.freedesktop.org
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index beb581b96ecd..d8741c35bbfc 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -226,6 +226,16 @@ config DRM_PANEL_SAMSUNG_LD9040
> depends on OF && SPI
> select VIDEOMODE_HELPERS
>
> +config DRM_PANEL_LGPHILIPS_SW43101
> + tristate "LG.Philips SW43101 DSI video mode panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for the LG.Philips SW43101
> FHD
> + (1080x1920) OLED DSI video mode panel found on the Xiaomi Mi Note 2.
> + To compile this driver as a module, choose M here.
> +
> config DRM_PANEL_LG_LB035Q02
> tristate "LG LB035Q024573 RGB panel"
> depends on GPIOLIB && OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index c8132050bcec..e79143ad14dd 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_PANEL_KHADAS_TS050) += panel-khadas-ts050.o
> obj-$(CONFIG_DRM_PANEL_KINGDISPLAY_KD097D04) += panel-kingdisplay-kd097d04.o
> obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
> obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> +obj-$(CONFIG_DRM_PANEL_LGPHILIPS_SW43101) += panel-lgphilips-sw43101.o
> obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
> obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
> diff --git a/drivers/gpu/drm/panel/panel-lgphilips-sw43101.c
> b/drivers/gpu/drm/panel/panel-lgphilips-sw43101.c
> new file mode 100644
> index ..b7adf1d4a8ab
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lgphilips-sw43101.c
> @@ -0,0 +1,363 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * LG.Philips SW43101 OLED Panel driver
> + * Generated with linux-mdss-dsi-panel-driver-generator
> + *
> + * Copyright (c) 2020 Yassine Oudjana
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +#include
> +
> +static const char * const regulator_names[] = {
> + "vdd",
> + "avdd",
> + "elvdd",
> + "elvss",
> +};
> +
> +struct sw43101_device {
> + struct drm_panel panel;
> + struct mipi_dsi_device *dsi;
> + struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
> + bool prepared;
> +};
> +
> +static inline
> +struct sw43101_device *to_sw43101_device(struct drm_panel *panel)
> +{
> + return container_of(panel, struct sw43101_device, panel);
> +}
> +
> +#define dsi_dcs_write_seq(dsi, seq...) do { \
> + static const u8 d[] = { seq }; \
> + int ret;\
> + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
> + if (ret < 0)\
> + return ret; \
> + } while (0)
> +
> +static void sw43101_reset(struct sw43101_device *ctx)
> +{
> + gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> + usleep_range(1, 11000);
> + gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> + usleep_range(1000,
[PATCH v2 03/13] drm/radeon: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Retriving the same information is less
efficient. Change to drm_display_info.is_hdmi
This is a TODO task in Documentation/gpu/todo.rst
Also, correct an inacurracy or bug in
radeon_connector_get_edid()/radeon_connector_free_edid(). Two variables
have EDID data:
- struct radeon_connector.edid
- struct drm_connector.edid_blob_ptr
The second is updated by calling drm_connector_update_edid_property() or
drm_get_edid() - which internally calls drm_connector_update_edid_property().
drm_display_info.is_hdmi is updated when this function is called.
radeon_connector_get_edid() calls drm_get_edid() to update
drm_connector.edid_blob_ptr/drm_display_info only in some cases. Change it
to be always up to date, so drm_display_info is always correct.
As counterpart, reset these values in radeon_connector_free_edid().
This second change is necessary for the previous one to work properly.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++---
drivers/gpu/drm/radeon/radeon_connectors.c | 15 +--
drivers/gpu/drm/radeon/radeon_display.c| 2 +-
drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++--
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 0fce73b9a646..844f61a36f20 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -713,7 +713,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_connector->use_digital &&
(radeon_connector->audio == RADEON_AUDIO_ENABLE))
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (connector->display_info.is_hdmi &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else if (radeon_connector->use_digital)
@@ -732,7 +732,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (connector->display_info.is_hdmi &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
@@ -756,7 +756,7 @@ atombios_get_encoder_mode(struct drm_encoder *encoder)
} else if (radeon_audio != 0) {
if (radeon_connector->audio == RADEON_AUDIO_ENABLE)
return ATOM_ENCODER_MODE_HDMI;
- else if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector)) &&
+ else if (connector->display_info.is_hdmi &&
(radeon_connector->audio == RADEON_AUDIO_AUTO))
return ATOM_ENCODER_MODE_HDMI;
else
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 607ad5620bd9..deaae181ac59 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -130,7 +130,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (radeon_connector->use_digital) {
- if
(drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+ if (connector->display_info.is_hdmi) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -138,7 +138,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
- if (drm_detect_hdmi_monitor(radeon_connector_edid(connector))) {
+ if (connector->display_info.is_hdmi) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -147,7 +147,7 @@ int radeon_get_monitor_bpc(struct drm_connector *connector)
dig_connector = radeon_connector->con_priv;
if ((dig_connector->dp_sink_type ==
CONNECTOR_OBJECT_ID_DISPLAYPORT) ||
(dig_connector->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) ||
- drm_detect_hdmi_monitor(radeon_connector_edid(connector)))
[PATCH v2 02/13] drm/vc4: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Once EDID is parsed, the monitor HDMI support information is available
through drm_display_info.is_hdmi. Use this value instead of calling
drm_detect_hdmi_monitor() to avoid a second parse.
This is a TODO task in Documentation/gpu/todo.rst
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index b4b4653fe301..d531e4c501eb 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -182,7 +182,8 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
drm_detect_hdmi_monitor(edid);
+ vc4_hdmi->encoder.hdmi_monitor =
+ connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -212,10 +213,9 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
if (!edid)
return -ENODEV;
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
+ vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
kfree(edid);
if (vc4_hdmi->disable_4kp60) {
--
2.33.0
[PATCH v2 00/13] replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Changelog: v2: - no helper function - A separate patch is made for amdgpu - zte patch is removed because that driver no longer exists [Why] Copy from Documentation/gpu/todo.rst === Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi --- Once EDID is parsed, the monitor HDMI support information is available through drm_display_info.is_hdmi. Many drivers still call drm_detect_hdmi_monitor() to retrieve the same information, which is less efficient. Audit each individual driver calling drm_detect_hdmi_monitor() and switch to drm_display_info.is_hdmi if applicable. = [How] I did it in two steps: - check that drm_display_info has a correct value. - in that case, replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi Almost all occurrences of drm_detect_hdmi_monitor() could be changed. Some small inconsistencies have been solved. Stats: drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c| 2 +- drivers/gpu/drm/drm_edid.c | 11 +-- drivers/gpu/drm/exynos/exynos_hdmi.c | 6 -- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 ++- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 6 -- drivers/gpu/drm/i915/display/intel_hdmi.c| 2 +- drivers/gpu/drm/i915/display/intel_sdvo.c| 3 ++- drivers/gpu/drm/msm/hdmi/hdmi_connector.c| 2 +- drivers/gpu/drm/nouveau/dispnv50/disp.c | 4 ++-- drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +--- drivers/gpu/drm/radeon/atombios_encoders.c | 6 +++--- drivers/gpu/drm/radeon/radeon_connectors.c | 15 +-- drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 ++-- drivers/gpu/drm/rockchip/rk3066_hdmi.c | 2 +- drivers/gpu/drm/sti/sti_hdmi.c | 10 ++ drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 4 ++-- drivers/gpu/drm/tegra/hdmi.c | 6 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 6 +++--- 22 files changed, 55 insertions(+), 55 deletions(-) Best regards. Claudio Suarez
[PATCH v2 01/13] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
According to the documentation, drm_add_edid_modes
"... Also fills out the _display_info structure and ELD in @connector
with any information which can be derived from the edid."
drm_add_edid_modes accepts a struct edid *edid parameter which may have a
value or may be null. When it is not null, connector->display_info and
connector->eld are updated according to the edid. When edid=NULL, only
connector->eld is reset. Reset connector->display_info to be consistent
and accurate.
Signed-off-by: Claudio Suarez
---
drivers/gpu/drm/drm_edid.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 6325877c5fd6..c643db17782c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5356,14 +5356,13 @@ int drm_add_edid_modes(struct drm_connector *connector,
struct edid *edid)
int num_modes = 0;
u32 quirks;
- if (edid == NULL) {
- clear_eld(connector);
- return 0;
- }
if (!drm_edid_is_valid(edid)) {
+ /* edid == NULL or invalid here */
clear_eld(connector);
- drm_warn(connector->dev, "%s: EDID invalid.\n",
-connector->name);
+ drm_reset_display_info(connector);
+ if (edid)
+ drm_warn(connector->dev, "%s: EDID invalid.\n",
+connector->name);
return 0;
}
--
2.33.0
Re: [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent
Hi Maxime, > drm/bridge: sn65dsi83: Switch to devm MIPI-DSI helpers > drm/bridge: sn65dsi83: Register and attach our DSI device at probe > drm/bridge: sn65dsi86: Switch to devm MIPI-DSI helpers > drm/bridge: sn65dsi86: Register and attach our DSI device at probe > drm/bridge: tc358775: Switch to devm MIPI-DSI helpers > drm/bridge: tc358775: Register and attach our DSI device at probe The above are: Acked-by: Sam Ravnborg I hope you can land this series soon. Sam
[PATCH 2/2] drm/bridge: parade-ps8640: Populate devices on aux-bus
Conventionally, panel is listed under the root in the device tree.
When userland asks for display mode, ps8640 bridge is responsible
for returning EDID when ps8640_bridge_get_edid() is called.
Now enable a new option of listing the panel under "aux-bus" of ps8640
bridge node in the device tree. In this case, panel driver can retrieve
EDID by triggering AUX transactions, without ps8640_bridge_get_edid()
calls at all.
To prevent the "old" and "new" options from interfering with each
other's logic flow, disable DRM_BRIDGE_OP_EDID when the new option
is taken.
Signed-off-by: Philip Chen
---
drivers/gpu/drm/bridge/parade-ps8640.c | 52 --
1 file changed, 40 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index acfe1bf0f936..98884f799ea8 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -14,6 +14,7 @@
#include
#include
+#include
#include
#include
#include
@@ -149,6 +150,24 @@ static inline struct ps8640 *aux_to_ps8640(struct
drm_dp_aux *aux)
return container_of(aux, struct ps8640, aux);
}
+static bool ps8640_of_panel_on_aux_bus(struct device *dev)
+{
+ struct device_node *bus, *panel;
+
+ if (!dev->of_node)
+ return false;
+
+ bus = of_get_child_by_name(dev->of_node, "aux-bus");
+ if (!bus)
+ return false;
+
+ panel = of_get_child_by_name(bus, "panel");
+ if (!panel)
+ return false;
+
+ return true;
+}
+
static void ps8640_ensure_hpd(struct ps8640 *ps_bridge)
{
struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
@@ -546,17 +565,6 @@ static int ps8640_probe(struct i2c_client *client)
if (!ps_bridge)
return -ENOMEM;
- /* port@1 is ps8640 output port */
- ret = drm_of_find_panel_or_bridge(np, 1, 0, , NULL);
- if (ret < 0)
- return ret;
- if (!panel)
- return -ENODEV;
-
- ps_bridge->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
- if (IS_ERR(ps_bridge->panel_bridge))
- return PTR_ERR(ps_bridge->panel_bridge);
-
ps_bridge->supplies[0].supply = "vdd33";
ps_bridge->supplies[1].supply = "vdd12";
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ps_bridge->supplies),
@@ -579,9 +587,16 @@ static int ps8640_probe(struct i2c_client *client)
ps_bridge->bridge.funcs = _bridge_funcs;
ps_bridge->bridge.of_node = dev->of_node;
- ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
ps_bridge->bridge.type = DRM_MODE_CONNECTOR_eDP;
+ /*
+* In the device tree, if panel is listed under aux-bus of the bridge
+* node, panel driver should be able to retrieve EDID by itself using
+* aux-bus. So let's not set DRM_BRIDGE_OP_EDID here.
+*/
+ if (!ps8640_of_panel_on_aux_bus(>dev))
+ ps_bridge->bridge.ops = DRM_BRIDGE_OP_EDID;
+
ps_bridge->page[PAGE0_DP_CNTL] = client;
ps_bridge->regmap[PAGE0_DP_CNTL] = devm_regmap_init_i2c(client,
ps8640_regmap_config);
@@ -615,6 +630,19 @@ static int ps8640_probe(struct i2c_client *client)
if (ret)
return ret;
+ devm_of_dp_aux_populate_ep_devices(_bridge->aux);
+
+ /* port@1 is ps8640 output port */
+ ret = drm_of_find_panel_or_bridge(np, 1, 0, , NULL);
+ if (ret < 0)
+ return ret;
+ if (!panel)
+ return -ENODEV;
+
+ ps_bridge->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
+ if (IS_ERR(ps_bridge->panel_bridge))
+ return PTR_ERR(ps_bridge->panel_bridge);
+
drm_bridge_add(_bridge->bridge);
return 0;
--
2.33.0.1079.g6e70778dc9-goog
[PATCH 1/2] drm/bridge: parade-ps8640: Enable runtime power management
Fit ps8640 driver into runtime power management framework:
First, break _poweron() to 3 parts: (1) turn on power and wait for
ps8640's internal MCU to finish init (2) check panel HPD (which is
proxy by GPIO9) (3) the other configs. Runtime _resume() can be called
before panel is powered, so we only add (1) to _resume() and do (2)(3)
in _pre_enable(). We also add (2) to _aux_transfer() as we want to
ensure panel HPD is asserted before we start AUX CH transactions.
The original driver has a mysterious delay of 50 ms between (2) and
(3). Since Parade's support can't explain what the delay is for, and we
don't see removing the delay break any boards at hand, remove it to fit
into this driver change.
Besides, rename "powered" to "pre_enabled" and don't check for it in
the pm_runtime calls. The pm_runtime calls are already refcounted so
there's no reason to check there. The other user of "powered",
_get_edid(), only cares if pre_enable() has already been called.
Lastly, change some existing DRM_...() logging to dev_...() along the
way, since DRM_...() seem to be deprecated in [1].
[1] https://patchwork.freedesktop.org/patch/454760/
Signed-off-by: Philip Chen
---
drivers/gpu/drm/bridge/parade-ps8640.c | 177 ++---
1 file changed, 103 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 3aaa90913bf8..acfe1bf0f936 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -100,7 +101,7 @@ struct ps8640 {
struct regulator_bulk_data supplies[2];
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_powerdown;
- bool powered;
+ bool pre_enabled;
};
static const struct regmap_config ps8640_regmap_config[] = {
@@ -148,6 +149,25 @@ static inline struct ps8640 *aux_to_ps8640(struct
drm_dp_aux *aux)
return container_of(aux, struct ps8640, aux);
}
+static void ps8640_ensure_hpd(struct ps8640 *ps_bridge)
+{
+ struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
+ struct device *dev = _bridge->page[PAGE2_TOP_CNTL]->dev;
+ int status;
+ int ret;
+
+ /*
+* Apparently something about the firmware in the chip signals that
+* HPD goes high by reporting GPIO9 as high (even though HPD isn't
+* actually connected to GPIO9).
+*/
+ ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
+ status & PS_GPIO9, 20 * 1000, 200 * 1000);
+
+ if (ret < 0)
+ dev_warn(dev, "HPD didn't go high: %d", ret);
+}
+
static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
{
@@ -171,6 +191,9 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
if (msg->address & ~SWAUX_ADDR_MASK)
return -EINVAL;
+ pm_runtime_get_sync(dev);
+ ps8640_ensure_hpd(ps_bridge);
+
switch (request) {
case DP_AUX_NATIVE_WRITE:
case DP_AUX_NATIVE_READ:
@@ -180,14 +203,15 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
case DP_AUX_I2C_READ:
break;
default:
- return -EINVAL;
+ ret = -EINVAL;
+ goto exit;
}
ret = regmap_write(map, PAGE0_AUXCH_CFG3, AUXCH_CFG3_RESET);
if (ret) {
DRM_DEV_ERROR(dev, "failed to write PAGE0_AUXCH_CFG3: %d\n",
ret);
- return ret;
+ goto exit;
}
/* Assume it's good */
@@ -213,7 +237,7 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
DRM_DEV_ERROR(dev,
"failed to write WDATA: %d\n",
ret);
- return ret;
+ goto exit;
}
}
}
@@ -228,7 +252,7 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
if (ret) {
DRM_DEV_ERROR(dev, "failed to read PAGE0_SWAUX_STATUS: %d\n",
ret);
- return ret;
+ goto exit;
}
switch (data & SWAUX_STATUS_MASK) {
@@ -250,9 +274,11 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
len = data & SWAUX_M_MASK;
break;
case SWAUX_STATUS_INVALID:
- return -EOPNOTSUPP;
+ ret = -EOPNOTSUPP;
+ goto exit;
case SWAUX_STATUS_TIMEOUT:
- return -ETIMEDOUT;
+ ret = -ETIMEDOUT;
+ goto exit;
}
if (len && (request == DP_AUX_NATIVE_READ ||
@@ -264,48 +290,48 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux
Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount
On Sat, Oct 16, 2021 at 12:44:50PM -0300, Jason Gunthorpe wrote: > Assuming changing FSDAX is hard.. How would DAX people feel about just > deleting the PUD/PMD support until it can be done with compound pages? I think there are customers who would find that an unacceptable answer :-)
Re: [PATCH 2/2] drm/msm/devfreq: Add 1ms delay before clamping freq
On 28/09/2021 00:04, Rob Clark wrote:
From: Rob Clark
Add a short delay before clamping to idle frequency on active->idle
transition. It takes ~0.5ms to increase the freq again on the next
idle->active transition, so this helps avoid extra freq transitions
on workloads that bounce between CPU and GPU.
Signed-off-by: Rob Clark
---
Note that this sort of re-introduces the theoretical race solved
by [1].. but that should not be a problem with something along the
lines of [2].
[1] https://patchwork.freedesktop.org/patch/455910/?series=95111=1
[2] https://patchwork.freedesktop.org/patch/455928/?series=95119=1
drivers/gpu/drm/msm/msm_gpu.h | 7 +
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 38 +--
2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 32a859307e81..2fcb6c195865 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -120,6 +120,13 @@ struct msm_gpu_devfreq {
* it is inactive.
*/
unsigned long idle_freq;
+
+ /**
+* idle_work:
+*
+* Used to delay clamping to idle freq on active->idle transition.
+*/
+ struct msm_hrtimer_work idle_work;
};
struct msm_gpu {
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 15b64f35c0f6..36e1930ee26d 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -96,8 +96,12 @@ static struct devfreq_dev_profile msm_devfreq_profile = {
.get_cur_freq = msm_devfreq_get_cur_freq,
};
+static void msm_devfreq_idle_work(struct kthread_work *work);
+
void msm_devfreq_init(struct msm_gpu *gpu)
{
+ struct msm_gpu_devfreq *df = >devfreq;
+
/* We need target support to do devfreq */
if (!gpu->funcs->gpu_busy)
return;
@@ -113,25 +117,27 @@ void msm_devfreq_init(struct msm_gpu *gpu)
msm_devfreq_profile.freq_table = NULL;
msm_devfreq_profile.max_state = 0;
- gpu->devfreq.devfreq = devm_devfreq_add_device(>pdev->dev,
+ df->devfreq = devm_devfreq_add_device(>pdev->dev,
_devfreq_profile, DEVFREQ_GOV_SIMPLE_ONDEMAND,
NULL);
- if (IS_ERR(gpu->devfreq.devfreq)) {
+ if (IS_ERR(df->devfreq)) {
DRM_DEV_ERROR(>pdev->dev, "Couldn't initialize GPU
devfreq\n");
- gpu->devfreq.devfreq = NULL;
+ df->devfreq = NULL;
return;
}
- devfreq_suspend_device(gpu->devfreq.devfreq);
+ devfreq_suspend_device(df->devfreq);
- gpu->cooling = of_devfreq_cooling_register(gpu->pdev->dev.of_node,
- gpu->devfreq.devfreq);
+ gpu->cooling = of_devfreq_cooling_register(gpu->pdev->dev.of_node,
df->devfreq);
if (IS_ERR(gpu->cooling)) {
DRM_DEV_ERROR(>pdev->dev,
"Couldn't register GPU cooling device\n");
gpu->cooling = NULL;
}
+
+ msm_hrtimer_work_init(>idle_work, gpu->worker,
msm_devfreq_idle_work,
+ CLOCK_MONOTONIC, HRTIMER_MODE_REL);
}
void msm_devfreq_cleanup(struct msm_gpu *gpu)
@@ -179,6 +185,11 @@ void msm_devfreq_active(struct msm_gpu *gpu)
unsigned int idle_time;
unsigned long target_freq = df->idle_freq;
+ /*
+* Cancel any pending transition to idle frequency:
+*/
+ hrtimer_cancel(>idle_work.timer);
+
/*
* Hold devfreq lock to synchronize with get_dev_status()/
* target() callbacks
@@ -209,9 +220,12 @@ void msm_devfreq_active(struct msm_gpu *gpu)
mutex_unlock(>devfreq->lock);
}
-void msm_devfreq_idle(struct msm_gpu *gpu)
+
+static void msm_devfreq_idle_work(struct kthread_work *work)
{
- struct msm_gpu_devfreq *df = >devfreq;
+ struct msm_gpu_devfreq *df = container_of(work,
+ struct msm_gpu_devfreq, idle_work.work);
+ struct msm_gpu *gpu = container_of(df, struct msm_gpu, devfreq);
unsigned long idle_freq, target_freq = 0;
/*
@@ -229,3 +243,11 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
mutex_unlock(>devfreq->lock);
}
+
+void msm_devfreq_idle(struct msm_gpu *gpu)
+{
+ struct msm_gpu_devfreq *df = >devfreq;
+
+ msm_hrtimer_queue_work(>idle_work, ms_to_ktime(1),
+ HRTIMER_MODE_ABS);
+}
--
2.31.1
Hi Rob,
I tested this patch on the OnePlus 6, with it I'm still able to reproduce the
crash introduced by
("drm/msm: Devfreq tuning").
Adjusting the delay from 1ms to 5ms seems to help, at least from some very
basic testing.
Perhaps the increased power reliability of the external power supply on dev boards is helping to mask the issue (hence
why it's harder to reproduce on db845c).
--
Kind Regards,
Caleb (they/them)
Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount
On Thu, Oct 14, 2021 at 06:37:35PM -0700, Dan Williams wrote: > On Thu, Oct 14, 2021 at 4:06 PM Jason Gunthorpe wrote: > > > > On Thu, Oct 14, 2021 at 12:01:14PM -0700, Dan Williams wrote: > > > > > Does anyone know why devmap is pte_special anyhow? > > > > > > It does not need to be special as mentioned here: > > > > > > https://lore.kernel.org/all/CAPcyv4iFeVDVPn6uc=aksyuvkiu3-fk-n16ijvzq3n8ot00...@mail.gmail.com/ > > > > I added a remark there > > > > Not special means more to me, it means devmap should do the refcounts > > properly like normal memory pages. > > > > It means vm_normal_page should return !NULL and it means insert_page, > > not insert_pfn should be used to install them in the PTE. VMAs should > > not be MIXED MAP, but normal struct page maps. > > > > I think this change alone would fix all the refcount problems > > everwhere in DAX and devmap. > > > > > The refcount dependencies also go away after this... > > > > > > https://lore.kernel.org/all/161604050866.1463742.7759521510383551055.st...@dwillia2-desk3.amr.corp.intel.com/ > > > > > > ...but you can see that patches 1 and 2 in that series depend on being > > > able to guarantee that all mappings are invalidated when the undelying > > > device that owns the pgmap goes away. > > > > If I have put everything together right this is because of what I > > pointed to here. FS-DAX is installing 0 refcount pages into PTEs and > > expecting that to work sanely. > > > > This means the page map cannot be removed until all the PTEs are fully > > flushed, which buggily doesn't happen because of the missing unplug. > > > > However, this is all because nobody incrd a refcount to represent the > > reference in the PTE and since this ment that 0 refcount pages were > > wrongly stuffed into PTEs then devmap used the refcount == 1 hack to > > unbreak GUP? > > > > So.. Is there some reason why devmap pages are trying so hard to avoid > > sane refcounting??? > > I wouldn't put it that way. It's more that the original sin of > ZONE_DEVICE that sought to reuse the lru field space, by never having > a zero recount, then got layered upon and calcified in malignant ways. > In the meantime surrounding infrastructure got decrustified. Work like > the 'struct page' cleanup among other things, made it clearer and > clearer over time that the original design choice needed to be fixed. So, there used to be some reason, but with the current code arrangement it is not the case? This is why it looks so strange when reading it.. AFIACT we are good on the LRU stuff now? Eg release_pages() does not use page->lru for is_zone_device_page()? > The MIXED_MAP and insert_pfn were a holdover from page-less DAX, but > now that we have page-available DAX, yes, we can skip the FS > notification and just rely on typical refcounting and hanging until > the FS has a chance to uninstall the PTEs. You're right, the FS > notification is an improvement to the conversion, not a requirement. It all sounds so simple. I looked at this for a good long time and the first major roadblock is huge pages. The mm side is designed around THP which puts a single high order page into the PUD/PMD such that the mm only has to juggle one page. This a very sane and reasonable thing to do. DAX is stuffing arrays of 4k pages into the PUD/PMDs. Aligning with THP would make using normal refconting much simpler. I looked at teaching the mm core to deal with page arrays - it is certainly doable, but it is quite inefficient and ugly mm code. So, can we fix DAX and TTM - the only uses of PUD/PMDs I could find? Joao has a series that does this to device-dax: https://lore.kernel.org/all/20210827145819.16471-1-joao.m.mart...@oracle.com/ TTM is kind of broken already but does have a struct page, and it is probably already a high order one. Maybe it is OK? I will ask Thomas That leaves FSDAX. Can this be fixed? I know nothing of filesystems or fsdax to guess. Sounds like folios to me .. Assuming changing FSDAX is hard.. How would DAX people feel about just deleting the PUD/PMD support until it can be done with compound pages? Doing so would allow fixing the lifecycle, cleaning up gup and basically delete a huge wack of slow DAX and devmap specific code from the mm. It also opens the door to removing the PTE flag and thus allowing DAX on more architectures. > However, there still needs to be something in the gup-fast path to > indicate that GUP_LONGTERM is not possible because the PTE > represents It looks easy now: 1) We know the pfn has a struct page * because it isn't pfn special 2) We can get a valid ref on the struct page *: head = try_grab_compound_head(page, 1, flags); Holding a ref ensures that head->pgmap is valid. 3) Then check the page type directly: if ((flags & FOLL_LONGTERM) && is_zone_device_page(head)) This tells us we can access the ZONE_DEVICE struct in the union 4) Ask what the pgmap owner wants to do: if (head->pgmap->deny_foll_longterm) return
Re: [PATCH v13 11/35] drm/tegra: dc: Support OPP and SoC core voltage scaling
01.10.2021 16:27, Ulf Hansson пишет:
> On Mon, 27 Sept 2021 at 00:42, Dmitry Osipenko wrote:
>>
>> Add OPP and SoC core voltage scaling support to the display controller
>> driver. This is required for enabling system-wide DVFS on pre-Tegra186
>> SoCs.
>>
>> Tested-by: Peter Geis # Ouya T30
>> Tested-by: Paul Fertser # PAZ00 T20
>> Tested-by: Nicolas Chauvet # PAZ00 T20 and TK1 T124
>> Tested-by: Matt Merhar # Ouya T30
>> Signed-off-by: Dmitry Osipenko
>> ---
>> drivers/gpu/drm/tegra/dc.c | 74 ++
>> drivers/gpu/drm/tegra/dc.h | 2 ++
>> 2 files changed, 76 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
>> index a29d64f87563..d4047a14e2b6 100644
>> --- a/drivers/gpu/drm/tegra/dc.c
>> +++ b/drivers/gpu/drm/tegra/dc.c
>> @@ -11,9 +11,12 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include
>> #include
>>
>> +#include
>> #include
>>
>> #include
>> @@ -1762,6 +1765,47 @@ int tegra_dc_state_setup_clock(struct tegra_dc *dc,
>> return 0;
>> }
>>
>> +static void tegra_dc_update_voltage_state(struct tegra_dc *dc,
>> + struct tegra_dc_state *state)
>> +{
>> + unsigned long rate, pstate;
>> + struct dev_pm_opp *opp;
>> + int err;
>> +
>> + if (!dc->has_opp_table)
>> + return;
>> +
>> + /* calculate actual pixel clock rate which depends on internal
>> divider */
>> + rate = DIV_ROUND_UP(clk_get_rate(dc->clk) * 2, state->div + 2);
>> +
>> + /* find suitable OPP for the rate */
>> + opp = dev_pm_opp_find_freq_ceil(dc->dev, );
>> +
>> + if (opp == ERR_PTR(-ERANGE))
>> + opp = dev_pm_opp_find_freq_floor(dc->dev, );
>> +
>> + if (IS_ERR(opp)) {
>> + dev_err(dc->dev, "failed to find OPP for %luHz: %pe\n",
>> + rate, opp);
>> + return;
>> + }
>> +
>> + pstate = dev_pm_opp_get_required_pstate(opp, 0);
>> + dev_pm_opp_put(opp);
>> +
>> + /*
>> +* The minimum core voltage depends on the pixel clock rate (which
>> +* depends on internal clock divider of the CRTC) and not on the
>> +* rate of the display controller clock. This is why we're not using
>> +* dev_pm_opp_set_rate() API and instead controlling the power domain
>> +* directly.
>> +*/
>> + err = dev_pm_genpd_set_performance_state(dc->dev, pstate);
>> + if (err)
>> + dev_err(dc->dev, "failed to set power domain state to %lu:
>> %d\n",
>> + pstate, err);
>
> Yeah, the above code looks very similar to the code I pointed to in
> patch6. Perhaps we need to discuss with Viresh, whether it makes sense
> to fold in a patch adding an opp helper function after all, to avoid
> the open coding.
>
> Viresh?
I'll keep it open-coded for now. This code is specific to Tegra because
normally ceil error shouldn't fall back to the floor, but for Tegra it's
expected to happen and it's a normal condition.
[PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare
All the panel driver check the fact that their prepare/unprepare
call was already called. It's not an ideal solution but fix
for now the problem on ili9881c
[ 9862.283296] [ cut here ]
[ 9862.288490] unbalanced disables for vcc3v3_lcd
[ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851
_regulator_disable+0xd4/0x190
from:
[ 9862.038619] drm_panel_unprepare+0x2c/0x4c
[ 9862.043212] panel_bridge_post_disable+0x18/0x24
[ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0
[ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
and:
[ 9862.183103] drm_panel_unprepare+0x2c/0x4c
[ 9862.187695] panel_bridge_post_disable+0x18/0x24
[ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
[ 9862.199117] disable_outputs+0x120/0x31c
Signed-off-by: Michael Trimarchi
---
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index 103a16018975..f75eecb0e65c 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -52,6 +52,8 @@ struct ili9881c {
struct regulator*power;
struct gpio_desc*reset;
+
+ boolprepared;
};
#define ILI9881C_SWITCH_PAGE_INSTR(_page) \
@@ -707,6 +709,10 @@ static int ili9881c_prepare(struct drm_panel *panel)
unsigned int i;
int ret;
+ /* Preparing when already prepared is a no-op */
+ if (ctx->prepared)
+ return 0;
+
/* Power the panel */
ret = regulator_enable(ctx->power);
if (ret)
@@ -745,6 +751,8 @@ static int ili9881c_prepare(struct drm_panel *panel)
if (ret)
return ret;
+ ctx->prepared = true;
+
return 0;
}
@@ -770,10 +778,16 @@ static int ili9881c_unprepare(struct drm_panel *panel)
{
struct ili9881c *ctx = panel_to_ili9881c(panel);
+ /* Unpreparing when already unprepared is a no-op */
+ if (!ctx->prepared)
+ return 0;
+
mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
regulator_disable(ctx->power);
gpiod_set_value(ctx->reset, 1);
+ ctx->prepared = false;
+
return 0;
}
--
2.25.1
Re: [PATCH 5/5] drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing
Hi Forget this one I can not replicate. We have another problem 9862.010474] Hardware name: Rockchip PX30 EVB (DT) [ 9862.015738] Call trace: [ 9862.018471] dump_backtrace+0x0/0x19c [ 9862.022586] show_stack+0x1c/0x70 [ 9862.026305] dump_stack_lvl+0x68/0x84 [ 9862.030408] dump_stack+0x1c/0x38 [ 9862.034125] ili9881c_unprepare+0x1c/0x4c [ 9862.038619] drm_panel_unprepare+0x2c/0x4c [ 9862.043212] panel_bridge_post_disable+0x18/0x24 [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0 [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0 [ 9862.060399] disable_outputs+0x120/0x31c [ 9862.064785] drm_atomic_helper_commit_tail_rpm+0x28/0xa0 [ 9862.070734] commit_tail+0xa4/0x184 [ 9862.074642] drm_atomic_helper_commit+0x164/0x37c [ 9862.079902] drm_atomic_commit+0x50/0x60 [ 9862.084298] drm_atomic_helper_disable_all+0x1fc/0x210 [ 9862.090053] drm_atomic_helper_shutdown+0x80/0x130 [ 9862.095419] rockchip_drm_platform_shutdown+0x1c/0x30 [ 9862.101081] platform_shutdown+0x28/0x40 [ 9862.105477] device_shutdown+0x15c/0x330 [ 9862.109877] __do_sys_reboot+0x214/0x294 [ 9862.114273] __arm64_sys_reboot+0x28/0x3c [ 9862.118765] invoke_syscall+0x48/0x114 [ 9862.122969] el0_svc_common.constprop.0+0x44/0xec [ 9862.128241] do_el0_svc+0x28/0x90 [ 9862.131958] el0_svc+0x20/0x60 [ 9862.135381] el0t_64_sync_handler+0x1a8/0x1b0 [ 9862.140263] el0t_64_sync+0x1a0/0x1a4 [ 9862.145769] CPU: 0 PID: 1 Comm: systemd-shutdow Tainted: GW 5.15.0-rc5 #1 [ 9862.154957] Hardware name: Rockchip PX30 EVB (DT) [ 9862.160221] Call trace: [ 9862.162954] dump_backtrace+0x0/0x19c [ 9862.167068] show_stack+0x1c/0x70 [ 9862.170787] dump_stack_lvl+0x68/0x84 [ 9862.174895] dump_stack+0x1c/0x38 [ 9862.178611] ili9881c_unprepare+0x1c/0x4c [ 9862.183103] drm_panel_unprepare+0x2c/0x4c [ 9862.187695] panel_bridge_post_disable+0x18/0x24 [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0 [ 9862.199117] disable_outputs+0x120/0x31c [ 9862.203512] drm_atomic_helper_commit_tail_rpm+0x28/0xa0 [ 9862.209461] commit_tail+0xa4/0x184 [ 9862.213368] drm_atomic_helper_commit+0x164/0x37c [ 9862.218629] drm_atomic_commit+0x50/0x60 [ 9862.223025] drm_atomic_helper_disable_all+0x1fc/0x210 [ 9862.228781] drm_atomic_helper_shutdown+0x80/0x130 [ 9862.234147] rockchip_drm_platform_shutdown+0x1c/0x30 [ 9862.239810] platform_shutdown+0x28/0x40 [ 9862.244205] device_shutdown+0x15c/0x330 [ 9862.248603] __do_sys_reboot+0x214/0x294 [ 9862.253000] __arm64_sys_reboot+0x28/0x3c [ 9862.257492] invoke_syscall+0x48/0x114 [ 9862.261696] el0_svc_common.constprop.0+0x44/0xec [ 9862.266970] do_el0_svc+0x28/0x90 [ 9862.270687] el0_svc+0x20/0x60 [ 9862.274111] el0t_64_sync_handler+0x1a8/0x1b0 [ 9862.278992] el0t_64_sync+0x1a0/0x1a4 [ 9862.283296] [ cut here ] [ 9862.288490] unbalanced disables for vcc3v3_lcd [ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851 _regulator_disable+0xd4/0x190 The panel can be disable two times. /* * TODO Only way found to call panel-bridge post_disable & * panel unprepare before the dsi "final" disable... * This needs to be fixed in the drm_bridge framework and the API * needs to be updated to manage our own call chains... */ if (dsi->panel_bridge->funcs->post_disable) dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); Is this comment relevant? Michael On Sat, Oct 16, 2021 at 3:32 PM Michael Nazzareno Trimarchi wrote: > > Hi Sam > > On Sat, Oct 16, 2021 at 2:25 PM Sam Ravnborg wrote: > > > > Hi Michael, > > > > I fail to follow the logic in this patch. > > > > > > On Sat, Oct 16, 2021 at 10:22:32AM +, Michael Trimarchi wrote: > > > The dsi registration is implemented in rockchip platform driver. > > > The attach can be called before the probe is terminated and therefore > > > we need to be sure that corresponding entry during attach is valid > > > > > > Signed-off-by: Michael Trimarchi > > > --- > > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++- > > > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 > > > include/drm/bridge/dw_mipi_dsi.h| 2 +- > > > 3 files changed, 16 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > index e44e18a0112a..44b211be15fc 100644 > > > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > > > @@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct > > > mipi_dsi_host *host, > > > dsi->device_found = true; > > > } > > > > > > + /* > > > + * NOTE: the dsi registration is implemented in > > > + * platform driver, that to say dsi would be exist after > > > + * probe is terminated. The call is done before the end of probe > > > + * so we need to
Re: [Freedreno] [PATCH 2/2] drm/msm/hdmi: switch to drm_bridge_connector
On Sat, 16 Oct 2021 at 01:25, wrote:
>
> Hi Dmitry
>
> On 2021-10-14 17:11, Dmitry Baryshkov wrote:
> > Merge old hdmi_bridge and hdmi_connector implementations. Use
> > drm_bridge_connector instead.
> >
> Can you please comment on the validation done on this change?
> Has basic bootup been verified on db820c as thats the only platform
> which shall use this.
Yes, this has been developed and validated on db820c
>
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/Makefile | 2 +-
> > drivers/gpu/drm/msm/hdmi/hdmi.c | 12 +-
> > drivers/gpu/drm/msm/hdmi/hdmi.h | 19 ++-
> > drivers/gpu/drm/msm/hdmi/hdmi_bridge.c| 81 -
> > .../msm/hdmi/{hdmi_connector.c => hdmi_hpd.c} | 154 ++
> > 5 files changed, 109 insertions(+), 159 deletions(-)
> > rename drivers/gpu/drm/msm/hdmi/{hdmi_connector.c => hdmi_hpd.c} (62%)
> >
> > diff --git a/drivers/gpu/drm/msm/Makefile
> > b/drivers/gpu/drm/msm/Makefile
> > index 904535eda0c4..91b09cda8a9c 100644
> > --- a/drivers/gpu/drm/msm/Makefile
> > +++ b/drivers/gpu/drm/msm/Makefile
> > @@ -19,7 +19,7 @@ msm-y := \
> > hdmi/hdmi.o \
> > hdmi/hdmi_audio.o \
> > hdmi/hdmi_bridge.o \
> > - hdmi/hdmi_connector.o \
> > + hdmi/hdmi_hpd.o \
> > hdmi/hdmi_i2c.o \
> > hdmi/hdmi_phy.o \
> > hdmi/hdmi_phy_8960.o \
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c
> > b/drivers/gpu/drm/msm/hdmi/hdmi.c
> > index db17a000d968..d1cf4df7188c 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> > @@ -8,6 +8,8 @@
> > #include
> > #include
> >
> > +#include
> > +
> > #include
> > #include "hdmi.h"
> >
> > @@ -41,7 +43,7 @@ static irqreturn_t msm_hdmi_irq(int irq, void
> > *dev_id)
> > struct hdmi *hdmi = dev_id;
> >
> > /* Process HPD: */
> > - msm_hdmi_connector_irq(hdmi->connector);
> > + msm_hdmi_hpd_irq(hdmi->bridge);
> >
> > /* Process DDC: */
> > msm_hdmi_i2c_irq(hdmi->i2c);
> > @@ -283,7 +285,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
> > goto fail;
> > }
> >
> > - hdmi->connector = msm_hdmi_connector_init(hdmi);
> > + hdmi->connector = drm_bridge_connector_init(hdmi->dev, encoder);
> > if (IS_ERR(hdmi->connector)) {
> > ret = PTR_ERR(hdmi->connector);
> > DRM_DEV_ERROR(dev->dev, "failed to create HDMI connector:
> > %d\n",
> > ret);
> > @@ -291,6 +293,8 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
> > goto fail;
> > }
> >
> > + drm_connector_attach_encoder(hdmi->connector, hdmi->encoder);
> > +
> > hdmi->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> > if (hdmi->irq < 0) {
> > ret = hdmi->irq;
> > @@ -307,7 +311,9 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
> > goto fail;
> > }
> >
> > - ret = msm_hdmi_hpd_enable(hdmi->connector);
> > + drm_bridge_connector_enable_hpd(hdmi->connector);
> > +
> > + ret = msm_hdmi_hpd_enable(hdmi->bridge);
> > if (ret < 0) {
> > DRM_DEV_ERROR(>pdev->dev, "failed to enable HPD: %d\n",
> > ret);
> > goto fail;
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h
> > b/drivers/gpu/drm/msm/hdmi/hdmi.h
> > index 82261078c6b1..736f348befb3 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> > @@ -114,6 +114,13 @@ struct hdmi_platform_config {
> > struct hdmi_gpio_data gpios[HDMI_MAX_NUM_GPIO];
> > };
> >
> > +struct hdmi_bridge {
> > + struct drm_bridge base;
> > + struct hdmi *hdmi;
> > + struct work_struct hpd_work;
> > +};
> > +#define to_hdmi_bridge(x) container_of(x, struct hdmi_bridge, base)
> > +
> > void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on);
> >
> > static inline void hdmi_write(struct hdmi *hdmi, u32 reg, u32 data)
> > @@ -230,13 +237,11 @@ void msm_hdmi_audio_set_sample_rate(struct hdmi
> > *hdmi, int rate);
> > struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi);
> > void msm_hdmi_bridge_destroy(struct drm_bridge *bridge);
> >
> > -/*
> > - * hdmi connector:
> > - */
> > -
> > -void msm_hdmi_connector_irq(struct drm_connector *connector);
> > -struct drm_connector *msm_hdmi_connector_init(struct hdmi *hdmi);
> > -int msm_hdmi_hpd_enable(struct drm_connector *connector);
> > +void msm_hdmi_hpd_irq(struct drm_bridge *bridge);
> > +enum drm_connector_status msm_hdmi_bridge_detect(
> > + struct drm_bridge *bridge);
> > +int msm_hdmi_hpd_enable(struct drm_bridge *bridge);
> > +void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge);
> >
> > /*
> > * i2c adapter for ddc:
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > index f04eb4a70f0d..211b73dddf65 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > @@ -5,17 +5,16 @@
> > */
>
Re: [PATCH v3 6/6] i915/display/dp: send a more fine-grained link-status uevent
Hi Simon,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on f6632721cd6231e1bf28b5317dcc7543e43359f7]
url:
https://github.com/0day-ci/linux/commits/Simon-Ser/drm-add-per-connector-hotplug-events/20211016-003611
base: f6632721cd6231e1bf28b5317dcc7543e43359f7
config: x86_64-randconfig-a006-20211015 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
a49f5386ce6b091da66ea7c3a1d9a588d53becf7)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/fb2b373f5b3b0f1e37e56270bf7aa0e6332f880d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Simon-Ser/drm-add-per-connector-hotplug-events/20211016-003611
git checkout fb2b373f5b3b0f1e37e56270bf7aa0e6332f880d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/i915/display/intel_dp.c:5267:2: error: implicit declaration
>> of function 'drm_sysfs_connector_status_event'
>> [-Werror,-Wimplicit-function-declaration]
drm_sysfs_connector_status_event(connector,
^
drivers/gpu/drm/i915/display/intel_dp.c:5267:2: note: did you mean
'drm_get_connector_status_name'?
include/drm/drm_connector.h:1729:13: note: 'drm_get_connector_status_name'
declared here
const char *drm_get_connector_status_name(enum drm_connector_status status);
^
1 error generated.
vim +/drm_sysfs_connector_status_event +5267
drivers/gpu/drm/i915/display/intel_dp.c
5245
5246 static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
5247 {
5248 struct intel_connector *intel_connector;
5249 struct drm_connector *connector;
5250
5251 intel_connector = container_of(work, typeof(*intel_connector),
5252 modeset_retry_work);
5253 connector = _connector->base;
5254 DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
5255connector->name);
5256
5257 /* Grab the locks before changing connector property*/
5258 mutex_lock(>dev->mode_config.mutex);
5259 /* Set connector link status to BAD and send a Uevent to notify
5260 * userspace to do a modeset.
5261 */
5262 drm_connector_set_link_status_property(connector,
5263
DRM_MODE_LINK_STATUS_BAD);
5264 mutex_unlock(>dev->mode_config.mutex);
5265 /* Send Hotplug uevent so userspace can reprobe */
5266 drm_kms_helper_hotplug_event(connector->dev);
> 5267 drm_sysfs_connector_status_event(connector,
5268
connector->dev->mode_config.link_status_property);
5269 }
5270
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 5/5] drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing
Hi Sam
On Sat, Oct 16, 2021 at 2:25 PM Sam Ravnborg wrote:
>
> Hi Michael,
>
> I fail to follow the logic in this patch.
>
>
> On Sat, Oct 16, 2021 at 10:22:32AM +, Michael Trimarchi wrote:
> > The dsi registration is implemented in rockchip platform driver.
> > The attach can be called before the probe is terminated and therefore
> > we need to be sure that corresponding entry during attach is valid
> >
> > Signed-off-by: Michael Trimarchi
> > ---
> > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++-
> > drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12
> > include/drm/bridge/dw_mipi_dsi.h| 2 +-
> > 3 files changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > index e44e18a0112a..44b211be15fc 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> > @@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct
> > mipi_dsi_host *host,
> > dsi->device_found = true;
> > }
> >
> > + /*
> > + * NOTE: the dsi registration is implemented in
> > + * platform driver, that to say dsi would be exist after
> > + * probe is terminated. The call is done before the end of probe
> > + * so we need to pass the dsi to the platform driver.
> > + */
> > if (pdata->host_ops && pdata->host_ops->attach) {
> > - ret = pdata->host_ops->attach(pdata->priv_data, device);
> > + ret = pdata->host_ops->attach(pdata->priv_data, device, dsi);
> > if (ret < 0)
> > return ret;
> > }
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > index a2262bee5aa4..32ddc8642ec1 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> > @@ -997,7 +997,8 @@ static const struct component_ops
> > dw_mipi_dsi_rockchip_ops = {
> > };
> >
> > static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
> > - struct mipi_dsi_device *device)
> > + struct mipi_dsi_device *device,
> > + struct dw_mipi_dsi *dmd)
> > {
> > struct dw_mipi_dsi_rockchip *dsi = priv_data;
> > struct device *second;
> > @@ -1005,6 +1006,8 @@ static int dw_mipi_dsi_rockchip_host_attach(void
> > *priv_data,
> >
> > mutex_lock(>usage_mutex);
> >
> > + dsi->dmd = dmd;
> > +
> > if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> > DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> > mutex_unlock(>usage_mutex);
> > @@ -1280,6 +1283,7 @@ static int dw_mipi_dsi_rockchip_probe(struct
> > platform_device *pdev)
> > {
> > struct device *dev = >dev;
> > struct device_node *np = dev->of_node;
> > + struct dw_mipi_dsi *dmd;
> > struct dw_mipi_dsi_rockchip *dsi;
> > struct phy_provider *phy_provider;
> > struct resource *res;
> > @@ -1391,9 +1395,9 @@ static int dw_mipi_dsi_rockchip_probe(struct
> > platform_device *pdev)
> > if (IS_ERR(phy_provider))
> > return PTR_ERR(phy_provider);
> >
> > - dsi->dmd = dw_mipi_dsi_probe(pdev, >pdata);
> > - if (IS_ERR(dsi->dmd)) {
> > - ret = PTR_ERR(dsi->dmd);
> > + dmd = dw_mipi_dsi_probe(pdev, >pdata);
> > + if (IS_ERR(dmd)) {
> > + ret = PTR_ERR(dmd);
>
> The memory pointed to by dmd is allocated in dw_mipi_dsi_probe(), but
> the pointer is not saved here.
> We rely on the attach operation to save the dmd pointer.
>
>
> In other words - the attach operation must be called before we call
> dw_mipi_dsi_rockchip_remove(), which uses the dmd member.
>
> This all looks wrong to me - are we papering over some other issue
Ok, it's wrong. I was not expecting that call.Anyway this was my path
on linux-next
dw_mipi_dsi_rockchip_probe
dw_mipi_dsi_probe -->start call
dw_mipi_dsi_rockchip_host_attach <-- this was not able to use dmd
dw_mipi_dsi_probe -> exit from the call
Michael
> here?
>
> Sam
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com
Re: [PATCH 0/5] Add support for Wanchanglong panel used in px30-evb v11
Hi Michael, On Sat, Oct 16, 2021 at 10:22:27AM +, Michael Trimarchi wrote: > This patch series add support for W552946ABA panel. This panel is used > in px30-evb v11. All the patches can be applied on top of drm-fixes > branch. The last patch is suppose to fix a race when the panel is built > in. Tested on px30 evb > > Michael Trimarchi (5): > dt-bindings: vendor-prefix: add Wanchanglong Electronics Technology > drm/panel: ilitek-ili9881d: add support for Wanchanglong W552946ABA > panel > dt-bindings: ili9881c: add compatible string for Wanchanglong > w552946aba > drm/panel: ilitek-ili9881c: Make gpio-reset optional The four patches has been applied to drm-misc-next. > drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing This patch looks like it does not belong in this series. Anyway - commented on it as I did not understand the code. Sam
Re: [PATCH 5/5] drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing
Hi Michael,
I fail to follow the logic in this patch.
On Sat, Oct 16, 2021 at 10:22:32AM +, Michael Trimarchi wrote:
> The dsi registration is implemented in rockchip platform driver.
> The attach can be called before the probe is terminated and therefore
> we need to be sure that corresponding entry during attach is valid
>
> Signed-off-by: Michael Trimarchi
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++-
> drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12
> include/drm/bridge/dw_mipi_dsi.h| 2 +-
> 3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index e44e18a0112a..44b211be15fc 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host
> *host,
> dsi->device_found = true;
> }
>
> + /*
> + * NOTE: the dsi registration is implemented in
> + * platform driver, that to say dsi would be exist after
> + * probe is terminated. The call is done before the end of probe
> + * so we need to pass the dsi to the platform driver.
> + */
> if (pdata->host_ops && pdata->host_ops->attach) {
> - ret = pdata->host_ops->attach(pdata->priv_data, device);
> + ret = pdata->host_ops->attach(pdata->priv_data, device, dsi);
> if (ret < 0)
> return ret;
> }
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index a2262bee5aa4..32ddc8642ec1 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -997,7 +997,8 @@ static const struct component_ops
> dw_mipi_dsi_rockchip_ops = {
> };
>
> static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
> - struct mipi_dsi_device *device)
> + struct mipi_dsi_device *device,
> + struct dw_mipi_dsi *dmd)
> {
> struct dw_mipi_dsi_rockchip *dsi = priv_data;
> struct device *second;
> @@ -1005,6 +1006,8 @@ static int dw_mipi_dsi_rockchip_host_attach(void
> *priv_data,
>
> mutex_lock(>usage_mutex);
>
> + dsi->dmd = dmd;
> +
> if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> mutex_unlock(>usage_mutex);
> @@ -1280,6 +1283,7 @@ static int dw_mipi_dsi_rockchip_probe(struct
> platform_device *pdev)
> {
> struct device *dev = >dev;
> struct device_node *np = dev->of_node;
> + struct dw_mipi_dsi *dmd;
> struct dw_mipi_dsi_rockchip *dsi;
> struct phy_provider *phy_provider;
> struct resource *res;
> @@ -1391,9 +1395,9 @@ static int dw_mipi_dsi_rockchip_probe(struct
> platform_device *pdev)
> if (IS_ERR(phy_provider))
> return PTR_ERR(phy_provider);
>
> - dsi->dmd = dw_mipi_dsi_probe(pdev, >pdata);
> - if (IS_ERR(dsi->dmd)) {
> - ret = PTR_ERR(dsi->dmd);
> + dmd = dw_mipi_dsi_probe(pdev, >pdata);
> + if (IS_ERR(dmd)) {
> + ret = PTR_ERR(dmd);
The memory pointed to by dmd is allocated in dw_mipi_dsi_probe(), but
the pointer is not saved here.
We rely on the attach operation to save the dmd pointer.
In other words - the attach operation must be called before we call
dw_mipi_dsi_rockchip_remove(), which uses the dmd member.
This all looks wrong to me - are we papering over some other issue
here?
Sam
Re: [PATCH] drm/i915: Prefer struct_size over open coded arithmetic
Hi Daniel and Jani, On Wed, Oct 13, 2021 at 01:51:30PM +0200, Daniel Vetter wrote: > On Wed, Oct 13, 2021 at 02:24:05PM +0300, Jani Nikula wrote: > > On Mon, 11 Oct 2021, Len Baker wrote: > > > Hi, > > > > > > On Sun, Oct 03, 2021 at 12:42:58PM +0200, Len Baker wrote: > > >> As noted in the "Deprecated Interfaces, Language Features, Attributes, > > >> and Conventions" documentation [1], size calculations (especially > > >> multiplication) should not be performed in memory allocator (or similar) > > >> function arguments due to the risk of them overflowing. This could lead > > >> to values wrapping around and a smaller allocation being made than the > > >> caller was expecting. Using those allocations could lead to linear > > >> overflows of heap memory and other misbehaviors. > > >> > > >> In this case these are not actually dynamic sizes: all the operands > > >> involved in the calculation are constant values. However it is better to > > >> refactor them anyway, just to keep the open-coded math idiom out of > > >> code. > > >> > > >> So, add at the end of the struct i915_syncmap a union with two flexible > > >> array members (these arrays share the same memory layout). This is > > >> possible using the new DECLARE_FLEX_ARRAY macro. And then, use the > > >> struct_size() helper to do the arithmetic instead of the argument > > >> "size + count * size" in the kmalloc and kzalloc() functions. > > >> > > >> Also, take the opportunity to refactor the __sync_seqno and __sync_child > > >> making them more readable. > > >> > > >> This code was detected with the help of Coccinelle and audited and fixed > > >> manually. > > >> > > >> [1] > > >> https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments > > >> > > >> Signed-off-by: Len Baker > > >> --- > > >> drivers/gpu/drm/i915/i915_syncmap.c | 12 > > >> 1 file changed, 8 insertions(+), 4 deletions(-) > > > > > > I received a mail telling that this patch doesn't build: > > > > > > == Series Details == > > > > > > Series: drm/i915: Prefer struct_size over open coded arithmetic > > > URL : https://patchwork.freedesktop.org/series/95408/ > > > State : failure > > > > > > But it builds without error against linux-next (tag next-20211001). > > > Against > > > which tree and branch do I need to build? > > > > drm-tip [1]. It's a sort of linux-next for graphics. I think there are > > still some branches that don't feed to linux-next. > > Yeah we need to get gt-next in linux-next asap. Joonas promised to send > out his patch to make that happen in dim. > -Daniel Is there a possibility that you give an "Acked-by" tag? And then this patch goes to the mainline through the Kees' tree or Gustavo's tree? Or is it better to wait for drm-tip to update? Regards, Len > > > > > BR, > > Jani. > > > > > > [1] https://cgit.freedesktop.org/drm/drm-tip > > > > > > > > > > Regards, > > > Len > > > > -- > > Jani Nikula, Intel Open Source Graphics Center > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
[PATCH 5/5] drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing
The dsi registration is implemented in rockchip platform driver.
The attach can be called before the probe is terminated and therefore
we need to be sure that corresponding entry during attach is valid
Signed-off-by: Michael Trimarchi
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +++-
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12
include/drm/bridge/dw_mipi_dsi.h| 2 +-
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index e44e18a0112a..44b211be15fc 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -362,8 +362,14 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host
*host,
dsi->device_found = true;
}
+ /*
+* NOTE: the dsi registration is implemented in
+* platform driver, that to say dsi would be exist after
+* probe is terminated. The call is done before the end of probe
+* so we need to pass the dsi to the platform driver.
+*/
if (pdata->host_ops && pdata->host_ops->attach) {
- ret = pdata->host_ops->attach(pdata->priv_data, device);
+ ret = pdata->host_ops->attach(pdata->priv_data, device, dsi);
if (ret < 0)
return ret;
}
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
index a2262bee5aa4..32ddc8642ec1 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
@@ -997,7 +997,8 @@ static const struct component_ops dw_mipi_dsi_rockchip_ops
= {
};
static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
- struct mipi_dsi_device *device)
+ struct mipi_dsi_device *device,
+ struct dw_mipi_dsi *dmd)
{
struct dw_mipi_dsi_rockchip *dsi = priv_data;
struct device *second;
@@ -1005,6 +1006,8 @@ static int dw_mipi_dsi_rockchip_host_attach(void
*priv_data,
mutex_lock(>usage_mutex);
+ dsi->dmd = dmd;
+
if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
mutex_unlock(>usage_mutex);
@@ -1280,6 +1283,7 @@ static int dw_mipi_dsi_rockchip_probe(struct
platform_device *pdev)
{
struct device *dev = >dev;
struct device_node *np = dev->of_node;
+ struct dw_mipi_dsi *dmd;
struct dw_mipi_dsi_rockchip *dsi;
struct phy_provider *phy_provider;
struct resource *res;
@@ -1391,9 +1395,9 @@ static int dw_mipi_dsi_rockchip_probe(struct
platform_device *pdev)
if (IS_ERR(phy_provider))
return PTR_ERR(phy_provider);
- dsi->dmd = dw_mipi_dsi_probe(pdev, >pdata);
- if (IS_ERR(dsi->dmd)) {
- ret = PTR_ERR(dsi->dmd);
+ dmd = dw_mipi_dsi_probe(pdev, >pdata);
+ if (IS_ERR(dmd)) {
+ ret = PTR_ERR(dmd);
if (ret != -EPROBE_DEFER)
DRM_DEV_ERROR(dev,
"Failed to probe dw_mipi_dsi: %d\n", ret);
diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
index bda8aa7c2280..cf81f19806ad 100644
--- a/include/drm/bridge/dw_mipi_dsi.h
+++ b/include/drm/bridge/dw_mipi_dsi.h
@@ -41,7 +41,7 @@ struct dw_mipi_dsi_phy_ops {
struct dw_mipi_dsi_host_ops {
int (*attach)(void *priv_data,
- struct mipi_dsi_device *dsi);
+ struct mipi_dsi_device *dsi, struct dw_mipi_dsi *dmd);
int (*detach)(void *priv_data,
struct mipi_dsi_device *dsi);
};
--
2.25.1
[PATCH 3/5] dt-bindings: ili9881c: add compatible string for Wanchanglong w552946aba
It utilizes an Ilitek ILI9881D controller chip, but its compatible with ili9881c so should be added to ilitek,ili9881c file. Add the compatible string for it. Signed-off-by: Michael Trimarchi --- .../devicetree/bindings/display/panel/ilitek,ili9881c.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml index b2fcec4f22fd..2d4a5643a785 100644 --- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml @@ -15,6 +15,7 @@ properties: - enum: - bananapi,lhr050h41 - feixin,k101-im2byl02 + - wanchanglong,w552946aba - const: ilitek,ili9881c backlight: true -- 2.25.1
[PATCH 4/5] drm/panel: ilitek-ili9881c: Make gpio-reset optional
Depends in how logic is connected to the board the gpio is
not stricly required.
Signed-off-by: Michael Trimarchi
---
.../devicetree/bindings/display/panel/ilitek,ili9881c.yaml | 1 -
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git
a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
index 2d4a5643a785..07789d554889 100644
--- a/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9881c.yaml
@@ -27,7 +27,6 @@ required:
- compatible
- power-supply
- reg
- - reset-gpios
additionalProperties: false
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index d1f20758ed08..103a16018975 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -883,7 +883,7 @@ static int ili9881c_dsi_probe(struct mipi_dsi_device *dsi)
return PTR_ERR(ctx->power);
}
- ctx->reset = devm_gpiod_get(>dev, "reset", GPIOD_OUT_LOW);
+ ctx->reset = devm_gpiod_get_optional(>dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(ctx->reset)) {
dev_err(>dev, "Couldn't get our reset GPIO\n");
return PTR_ERR(ctx->reset);
--
2.25.1
[PATCH 2/5] drm/panel: ilitek-ili9881d: add support for Wanchanglong W552946ABA panel
W552946ABA is a panel by Wanchanglong. This panel utilizes the Ilitek ILI9881D
controller.
Add this panel's initialzation sequence and timing to ILI9881D driver.
Tested on px30-evb v11
Signed-off-by: Michael Trimarchi
---
drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 238 +-
1 file changed, 237 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
index 0145129d7c66..d1f20758ed08 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
@@ -42,6 +42,7 @@ struct ili9881c_desc {
const struct ili9881c_instr *init;
const size_t init_length;
const struct drm_display_mode *mode;
+ const unsigned long mode_flags;
};
struct ili9881c {
@@ -453,6 +454,213 @@ static const struct ili9881c_instr k101_im2byl02_init[] =
{
ILI9881C_COMMAND_INSTR(0xD3, 0x3F), /* VN0 */
};
+static const struct ili9881c_instr w552946ab_init[] = {
+ ILI9881C_SWITCH_PAGE_INSTR(3),
+ ILI9881C_COMMAND_INSTR(0x01, 0x00),
+ ILI9881C_COMMAND_INSTR(0x02, 0x00),
+ ILI9881C_COMMAND_INSTR(0x03, 0x53),
+ ILI9881C_COMMAND_INSTR(0x04, 0x53),
+ ILI9881C_COMMAND_INSTR(0x05, 0x13),
+ ILI9881C_COMMAND_INSTR(0x06, 0x04),
+ ILI9881C_COMMAND_INSTR(0x07, 0x02),
+ ILI9881C_COMMAND_INSTR(0x08, 0x02),
+ ILI9881C_COMMAND_INSTR(0x09, 0x00),
+ ILI9881C_COMMAND_INSTR(0x0A, 0x00),
+ ILI9881C_COMMAND_INSTR(0x0B, 0x00),
+ ILI9881C_COMMAND_INSTR(0x0C, 0x00),
+ ILI9881C_COMMAND_INSTR(0x0D, 0x00),
+ ILI9881C_COMMAND_INSTR(0x0E, 0x00),
+ ILI9881C_COMMAND_INSTR(0x0F, 0x00),
+
+ ILI9881C_COMMAND_INSTR(0x10, 0x00),
+ ILI9881C_COMMAND_INSTR(0x11, 0x00),
+ ILI9881C_COMMAND_INSTR(0x12, 0x00),
+ ILI9881C_COMMAND_INSTR(0x13, 0x00),
+ ILI9881C_COMMAND_INSTR(0x14, 0x00),
+ ILI9881C_COMMAND_INSTR(0x15, 0x08),
+ ILI9881C_COMMAND_INSTR(0x16, 0x10),
+ ILI9881C_COMMAND_INSTR(0x17, 0x00),
+ ILI9881C_COMMAND_INSTR(0x18, 0x08),
+ ILI9881C_COMMAND_INSTR(0x19, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1A, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1B, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1C, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1D, 0x00),
+ ILI9881C_COMMAND_INSTR(0x1E, 0xC0),
+ ILI9881C_COMMAND_INSTR(0x1F, 0x80),
+
+ ILI9881C_COMMAND_INSTR(0x20, 0x02),
+ ILI9881C_COMMAND_INSTR(0x21, 0x09),
+ ILI9881C_COMMAND_INSTR(0x22, 0x00),
+ ILI9881C_COMMAND_INSTR(0x23, 0x00),
+ ILI9881C_COMMAND_INSTR(0x24, 0x00),
+ ILI9881C_COMMAND_INSTR(0x25, 0x00),
+ ILI9881C_COMMAND_INSTR(0x26, 0x00),
+ ILI9881C_COMMAND_INSTR(0x27, 0x00),
+ ILI9881C_COMMAND_INSTR(0x28, 0x55),
+ ILI9881C_COMMAND_INSTR(0x29, 0x03),
+ ILI9881C_COMMAND_INSTR(0x2A, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2B, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2C, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2D, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2E, 0x00),
+ ILI9881C_COMMAND_INSTR(0x2F, 0x00),
+
+ ILI9881C_COMMAND_INSTR(0x30, 0x00),
+ ILI9881C_COMMAND_INSTR(0x31, 0x00),
+ ILI9881C_COMMAND_INSTR(0x32, 0x00),
+ ILI9881C_COMMAND_INSTR(0x33, 0x00),
+ ILI9881C_COMMAND_INSTR(0x34, 0x04),
+ ILI9881C_COMMAND_INSTR(0x35, 0x05),
+ ILI9881C_COMMAND_INSTR(0x36, 0x05),
+ ILI9881C_COMMAND_INSTR(0x37, 0x00),
+ ILI9881C_COMMAND_INSTR(0x38, 0x3C),
+ ILI9881C_COMMAND_INSTR(0x39, 0x35),
+ ILI9881C_COMMAND_INSTR(0x3A, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3B, 0x40),
+ ILI9881C_COMMAND_INSTR(0x3C, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3D, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3E, 0x00),
+ ILI9881C_COMMAND_INSTR(0x3F, 0x00),
+
+ ILI9881C_COMMAND_INSTR(0x40, 0x00),
+ ILI9881C_COMMAND_INSTR(0x41, 0x88),
+ ILI9881C_COMMAND_INSTR(0x42, 0x00),
+ ILI9881C_COMMAND_INSTR(0x43, 0x00),
+ ILI9881C_COMMAND_INSTR(0x44, 0x1F),
+
+ ILI9881C_COMMAND_INSTR(0x50, 0x01),
+ ILI9881C_COMMAND_INSTR(0x51, 0x23),
+ ILI9881C_COMMAND_INSTR(0x52, 0x45),
+ ILI9881C_COMMAND_INSTR(0x53, 0x67),
+ ILI9881C_COMMAND_INSTR(0x54, 0x89),
+ ILI9881C_COMMAND_INSTR(0x55, 0xaB),
+ ILI9881C_COMMAND_INSTR(0x56, 0x01),
+ ILI9881C_COMMAND_INSTR(0x57, 0x23),
+ ILI9881C_COMMAND_INSTR(0x58, 0x45),
+ ILI9881C_COMMAND_INSTR(0x59, 0x67),
+ ILI9881C_COMMAND_INSTR(0x5A, 0x89),
+ ILI9881C_COMMAND_INSTR(0x5B, 0xAB),
+ ILI9881C_COMMAND_INSTR(0x5C, 0xCD),
+ ILI9881C_COMMAND_INSTR(0x5D, 0xEF),
+ ILI9881C_COMMAND_INSTR(0x5E, 0x03),
+ ILI9881C_COMMAND_INSTR(0x5F, 0x14),
+
+ ILI9881C_COMMAND_INSTR(0x60, 0x15),
+ ILI9881C_COMMAND_INSTR(0x61, 0x0C),
+ ILI9881C_COMMAND_INSTR(0x62, 0x0D),
+ ILI9881C_COMMAND_INSTR(0x63, 0x0E),
+ ILI9881C_COMMAND_INSTR(0x64, 0x0F),
+
[PATCH 1/5] dt-bindings: vendor-prefix: add Wanchanglong Electronics Technology
Wanchanglong Electronics Technology is a company to provide LCD modules. Signed-off-by: Michael Trimarchi --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index a867f7102c35..5c43391d8c3d 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1304,6 +1304,8 @@ patternProperties: description: Wondermedia Technologies, Inc. "^wobo,.*": description: Wobo + "^wanchanglong,.*": +description: Wanchanglong Electronics Technology（SHENZHEN）Co.，Ltd. "^x-powers,.*": description: X-Powers "^xes,.*": -- 2.25.1
[PATCH 0/5] Add support for Wanchanglong panel used in px30-evb v11
This patch series add support for W552946ABA panel. This panel is used in px30-evb v11. All the patches can be applied on top of drm-fixes branch. The last patch is suppose to fix a race when the panel is built in. Tested on px30 evb Michael Trimarchi (5): dt-bindings: vendor-prefix: add Wanchanglong Electronics Technology drm/panel: ilitek-ili9881d: add support for Wanchanglong W552946ABA panel dt-bindings: ili9881c: add compatible string for Wanchanglong w552946aba drm/panel: ilitek-ili9881c: Make gpio-reset optional drm/bridge: dw-mipi-dsi: Fix dsi registration during drm probing .../display/panel/ilitek,ili9881c.yaml| 2 +- .../devicetree/bindings/vendor-prefixes.yaml | 2 + drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 8 +- drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 240 +- .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 +- include/drm/bridge/dw_mipi_dsi.h | 2 +- 6 files changed, 257 insertions(+), 9 deletions(-) -- 2.25.1
Re: [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver
On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote: > > > On 2021-10-15 07:37, Claudio Suarez wrote: > > a) Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. The amdgpu driver still calls > > drm_detect_hdmi_monitor() to retrieve the same information, which > > is less efficient. Change to drm_display_info.is_hdmi > > > > This is a TODO task in Documentation/gpu/todo.rst > > > > b) drm_display_info is updated by drm_get_edid() or > > drm_connector_update_edid_property(). In the amdgpu driver it is almost > > always updated when the edid is read in amdgpu_connector_get_edid(), > > but not always. Change amdgpu_connector_get_edid() and > > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows > > a) > > to work properly. > > > > c) Use drm_edid_get_monitor_name() instead of duplicating the code that > > parses the EDID in dm_helpers_parse_edid_caps() > > > > Also, remove the unused "struct dc_context *ctx" parameter in > > dm_helpers_parse_edid_caps() > > > > Thanks for this work. > > The fact that you listed three separate changes in this commit > is a clear indication that this patch should be three separate > patches instead. Separating the functional bits from the straight > refactor will help with bisection if this leads to a regression. > > All changes look reasonable to me, though. With this patch split > into three patches in the sequence (b), (c), then (a) this is > Reviewed-by: Harry Wentland Ok, thanks. I'll send three patches. BR Claudio Suarez
Re: [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent
Hi Maxime, > drm/bridge: adv7533: Switch to devm MIPI-DSI helpers > drm/bridge: adv7511: Register and attach our DSI device at probe > drm/bridge: anx7625: Switch to devm MIPI-DSI helpers > drm/bridge: anx7625: Register and attach our DSI device at probe > drm/bridge: lt8912b: Switch to devm MIPI-DSI helpers > drm/bridge: lt8912b: Register and attach our DSI device at probe > drm/bridge: lt9611: Switch to devm MIPI-DSI helpers > drm/bridge: lt9611: Register and attach our DSI device at probe > drm/bridge: lt9611uxc: Switch to devm MIPI-DSI helpers > drm/bridge: lt9611uxc: Register and attach our DSI device at probe > drm/bridge: ps8640: Switch to devm MIPI-DSI helpers > drm/bridge: ps8640: Register and attach our DSI device at probe All the above are: Acked-by: Sam Ravnborg Will try to look at sn65dsi83 and tc358775 later today. I assume kirin and exynos is already covered. Sam
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
On Sat, Oct 16, 2021 at 10:25:03AM +0200, Claudio Suarez wrote:
> On Fri, Oct 15, 2021 at 10:33:29PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 15, 2021 at 09:24:06PM +0200, Claudio Suarez wrote:
> > > On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > > > > According to the documentation, drm_add_edid_modes
> > > > > "... Also fills out the _display_info structure and ELD in
> > > > > @connector
> > > > > with any information which can be derived from the edid."
> > > > >
> > > > > drm_add_edid_modes accepts a struct edid *edid parameter which may
> > > > > have a
> > > > > value or may be null. When it is not null, connector->display_info and
> > > > > connector->eld are updated according to the edid. When edid=NULL, only
> > > > > connector->eld is reset. Reset connector->display_info to be
> > > > > consistent
> > > > > and accurate.
> > > > >
> > > > > Signed-off-by: Claudio Suarez
> > > > > ---
> > > > > drivers/gpu/drm/drm_edid.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > > index 6325877c5fd6..6cbe09b2357c 100644
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector
> > > > > *connector, struct edid *edid)
> > > > >
> > > > > if (edid == NULL) {
> > > > > clear_eld(connector);
> > > > > + drm_reset_display_info(connector);
> > > > > return 0;
> > > > > }
> > > > > if (!drm_edid_is_valid(edid)) {
> > > > > clear_eld(connector);
> > > > > + drm_reset_display_info(connector);
> > > >
> > > > Looks easier if you pull both of those out from these branches and
> > > > just call them unconditionally at the start.
> > >
> > > After looking at the full code, I am not sure. This is the code:
> > > ==
> > > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> > > {
> > > int num_modes = 0;
> > > u32 quirks;
> > >
> > > if (edid == NULL) {
> > > clear_eld(connector);
> > > drm_reset_display_info(connector); <--- added by me
> > > return 0;
> > > }
> > > if (!drm_edid_is_valid(edid)) {
> > > clear_eld(connector);
> > > drm_reset_display_info(connector); <--- added by me
> > > drm_warn(connector->dev, "%s: EDID invalid.\n",
> > > connector->name);
> > > return 0;
> > > }
> > >
> > > drm_edid_to_eld(connector, edid);
> > >
> > > quirks = drm_add_display_info(connector, edid);
> > > etc...
> > > =
> > >
> > > If we move those out of these branches and edid != NULL, we are executing
> > > an
> > > unnecessary clear_eld(connector) and an unnecessary
> > > drm_reset_display_info(connector)
> > > because the fields will be set in the next drm_edid_to_eld(connector,
> > > edid) and
> > > drm_add_display_info(connector, edid)
> > >
> > > Do we want this ?
> >
> > Seems fine by me. And maybe we could nuke the second
> > drm_reset_display_info() from deeper inside drm_add_display_info()?
> > Not sure if drm_add_display_info() still has to be able to operate
> > standalone or not.
> >
> > Hmm. Another option is to just move all these NULL/invalid edid
> > checks into drm_edid_to_eld() and drm_add_display_info().
>
> I was thinking about this. We can use a boolean variable:
> ===
> bool edid_is_invalid;
>
> edid_is_invalid = !drm_edid_is_valid(edid);
>
> if (edid == NULL || edid_is_invalid) {
> clear_eld(connector);
> drm_reset_display_info(connector);
> if (edid_is_invalid)
> drm_warn(connector->dev, "%s: EDID invalid.\n",
> connector->name);
> return 0;
> }
>
> drm_edid_to_eld(connector, edid);
> ...
> ===
> Internally, drm_edid_is_valid() handles NULL pointers properly.
> It is a quite elegant solution with a small change in the original
> design, and it improves this part in the way you pointed out.
I'll send a patch with this idea and we can talk about the new code.
Thanks!
Best regards,
Claudio Suarez.
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
On Fri, Oct 15, 2021 at 10:33:29PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 09:24:06PM +0200, Claudio Suarez wrote:
> > On Fri, Oct 15, 2021 at 03:03:13PM +0300, Ville Syrjälä wrote:
> > > On Fri, Oct 15, 2021 at 01:36:59PM +0200, Claudio Suarez wrote:
> > > > According to the documentation, drm_add_edid_modes
> > > > "... Also fills out the _display_info structure and ELD in
> > > > @connector
> > > > with any information which can be derived from the edid."
> > > >
> > > > drm_add_edid_modes accepts a struct edid *edid parameter which may have
> > > > a
> > > > value or may be null. When it is not null, connector->display_info and
> > > > connector->eld are updated according to the edid. When edid=NULL, only
> > > > connector->eld is reset. Reset connector->display_info to be consistent
> > > > and accurate.
> > > >
> > > > Signed-off-by: Claudio Suarez
> > > > ---
> > > > drivers/gpu/drm/drm_edid.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > index 6325877c5fd6..6cbe09b2357c 100644
> > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > @@ -5358,10 +5358,12 @@ int drm_add_edid_modes(struct drm_connector
> > > > *connector, struct edid *edid)
> > > >
> > > > if (edid == NULL) {
> > > > clear_eld(connector);
> > > > + drm_reset_display_info(connector);
> > > > return 0;
> > > > }
> > > > if (!drm_edid_is_valid(edid)) {
> > > > clear_eld(connector);
> > > > + drm_reset_display_info(connector);
> > >
> > > Looks easier if you pull both of those out from these branches and
> > > just call them unconditionally at the start.
> >
> > After looking at the full code, I am not sure. This is the code:
> > ==
> > int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
> > {
> > int num_modes = 0;
> > u32 quirks;
> >
> > if (edid == NULL) {
> > clear_eld(connector);
> > drm_reset_display_info(connector); <--- added by me
> > return 0;
> > }
> > if (!drm_edid_is_valid(edid)) {
> > clear_eld(connector);
> > drm_reset_display_info(connector); <--- added by me
> > drm_warn(connector->dev, "%s: EDID invalid.\n",
> > connector->name);
> > return 0;
> > }
> >
> > drm_edid_to_eld(connector, edid);
> >
> > quirks = drm_add_display_info(connector, edid);
> > etc...
> > =
> >
> > If we move those out of these branches and edid != NULL, we are executing an
> > unnecessary clear_eld(connector) and an unnecessary
> > drm_reset_display_info(connector)
> > because the fields will be set in the next drm_edid_to_eld(connector, edid)
> > and
> > drm_add_display_info(connector, edid)
> >
> > Do we want this ?
>
> Seems fine by me. And maybe we could nuke the second
> drm_reset_display_info() from deeper inside drm_add_display_info()?
> Not sure if drm_add_display_info() still has to be able to operate
> standalone or not.
>
> Hmm. Another option is to just move all these NULL/invalid edid
> checks into drm_edid_to_eld() and drm_add_display_info().
I was thinking about this. We can use a boolean variable:
===
bool edid_is_invalid;
edid_is_invalid = !drm_edid_is_valid(edid);
if (edid == NULL || edid_is_invalid) {
clear_eld(connector);
drm_reset_display_info(connector);
if (edid_is_invalid)
drm_warn(connector->dev, "%s: EDID invalid.\n",
connector->name);
return 0;
}
drm_edid_to_eld(connector, edid);
...
===
Internally, drm_edid_is_valid() handles NULL pointers properly.
It is a quite elegant solution with a small change in the original
design, and it improves this part in the way you pointed out.
Best regards,
Claudio Suarez
Re: [RFC PATCH] drm/panel: ilitek-ili9881d: add support for Wanchanglong W552946ABA panel
Hi
On Fri, Oct 15, 2021 at 9:15 PM Sam Ravnborg wrote:
>
> Hi Michael,
>
> > Add this panel's initialzation sequence and timing to ILI9881D driver.
> > Tested on px30-evb v11
> Patch looks good, but we need the vendor and the compatible documented.
>
> >
> > Signed-off-by: Michael Trimarchi
> > ---
> > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 238 +-
> > 1 file changed, 237 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > index 0145129d7c66..cf53b43e0907 100644
> > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > @@ -42,6 +42,7 @@ struct ili9881c_desc {
> > const struct ili9881c_instr *init;
> > const size_t init_length;
> > const struct drm_display_mode *mode;
> > + const unsigned long mode_flags;
> > };
> >
> > struct ili9881c {
> > @@ -453,6 +454,213 @@ static const struct ili9881c_instr
> > k101_im2byl02_init[] = {
> > ILI9881C_COMMAND_INSTR(0xD3, 0x3F), /* VN0 */
> > };
> >
>
> If you by any chance could comment a little on what goes on that would
> be nice.
> > +static const struct ili9881c_instr w552946ab_init[] = {
> > + ILI9881C_SWITCH_PAGE_INSTR(3),
> > + ILI9881C_COMMAND_INSTR(0x01, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x02, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x03, 0x53),
> > + ILI9881C_COMMAND_INSTR(0x04, 0x53),
> > + ILI9881C_COMMAND_INSTR(0x05, 0x13),
> > + ILI9881C_COMMAND_INSTR(0x06, 0x04),
> > + ILI9881C_COMMAND_INSTR(0x07, 0x02),
> > + ILI9881C_COMMAND_INSTR(0x08, 0x02),
> > + ILI9881C_COMMAND_INSTR(0x09, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x0A, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x0B, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x0C, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x0D, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x0E, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x0F, 0x00),
> > +
> > + ILI9881C_COMMAND_INSTR(0x10, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x11, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x12, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x13, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x14, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x15, 0x08),
> > + ILI9881C_COMMAND_INSTR(0x16, 0x10),
> > + ILI9881C_COMMAND_INSTR(0x17, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x18, 0x08),
> > + ILI9881C_COMMAND_INSTR(0x19, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x1A, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x1B, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x1C, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x1D, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x1E, 0xC0),
> > + ILI9881C_COMMAND_INSTR(0x1F, 0x80),
> > +
> > + ILI9881C_COMMAND_INSTR(0x20, 0x02),
> > + ILI9881C_COMMAND_INSTR(0x21, 0x09),
> > + ILI9881C_COMMAND_INSTR(0x22, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x23, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x24, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x25, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x26, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x27, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x28, 0x55),
> > + ILI9881C_COMMAND_INSTR(0x29, 0x03),
> > + ILI9881C_COMMAND_INSTR(0x2A, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x2B, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x2C, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x2D, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x2E, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x2F, 0x00),
> > +
> > + ILI9881C_COMMAND_INSTR(0x30, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x31, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x32, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x33, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x34, 0x04),
> > + ILI9881C_COMMAND_INSTR(0x35, 0x05),
> > + ILI9881C_COMMAND_INSTR(0x36, 0x05),
> > + ILI9881C_COMMAND_INSTR(0x37, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x38, 0x3C),
> > + ILI9881C_COMMAND_INSTR(0x39, 0x35),
> > + ILI9881C_COMMAND_INSTR(0x3A, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x3B, 0x40),
> > + ILI9881C_COMMAND_INSTR(0x3C, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x3D, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x3E, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x3F, 0x00),
> > +
> > + ILI9881C_COMMAND_INSTR(0x40, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x41, 0x88),
> > + ILI9881C_COMMAND_INSTR(0x42, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x43, 0x00),
> > + ILI9881C_COMMAND_INSTR(0x44, 0x1F),
> > +
> > + ILI9881C_COMMAND_INSTR(0x50, 0x01),
> > + ILI9881C_COMMAND_INSTR(0x51, 0x23),
> > + ILI9881C_COMMAND_INSTR(0x52, 0x45),
> > + ILI9881C_COMMAND_INSTR(0x53, 0x67),
> > + ILI9881C_COMMAND_INSTR(0x54, 0x89),
> > + ILI9881C_COMMAND_INSTR(0x55, 0xaB),
> > + ILI9881C_COMMAND_INSTR(0x56, 0x01),
> > + ILI9881C_COMMAND_INSTR(0x57, 0x23),
> > + ILI9881C_COMMAND_INSTR(0x58, 0x45),
> > + ILI9881C_COMMAND_INSTR(0x59, 0x67),
> > + ILI9881C_COMMAND_INSTR(0x5A, 0x89),
> > +
