Re: [Freedreno] [PATCH] drm/msm/dp: Only create debugfs for PRIMARY minor
On Fri 15 Oct 18:44 CDT 2021, abhin...@codeaurora.org wrote: > On 2021-10-15 16:13, Bjorn Andersson wrote: > > dpu_kms_debugfs_init() and hence dp_debug_get() gets invoked for each > > minor being registered. But dp_debug_get() will allocate a new struct > > dp_debug for each call and this will be associated as dp->debug. > > > > As such dp_debug will create debugfs files in both the PRIMARY and the > > RENDER minor's debugfs directory, but only the last reference will be > > remembered. > > > > The only use of this reference today is in the cleanup path in > > dp_display_deinit_sub_modules() and the dp_debug_private object does > > outlive the debugfs entries in either case, so there doesn't seem to be > > any adverse effects of this, but per the code the current behavior is > > unexpected, so change it to only create dp_debug for the PRIMARY minor. > > > > If i understand correctly, today because of this, we get redundant debugfs > nodes right? > > /sys/kernel/debug/dri//dp_debug > /sys/kernel/debug/dri//dp_debug > > Both of these will hold the same information as they are for the same DP > controller right? That's correct, all the dp_debug debugfs files end up in both minors. The problem is that dp->debug points to the dp_debug struct for one of them, which afaict doesn't have any really bad effects today - unless you try to clean up the dp_debug state, but that will clear out the entire dri/... > In that case, this is true even for the other DPU kms information too. > > Why not move this check one level up to dpu_kms_debugfs_init? > I was expecting that perhaps some of this information was minor-specific, but if that isn't the case it sounds reasonable that we should push this one step up. Regards, Bjorn > > Signed-off-by: Bjorn Andersson > > --- > > drivers/gpu/drm/msm/dp/dp_display.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > > b/drivers/gpu/drm/msm/dp/dp_display.c > > index 3aa67c53dbc0..06773b58bb60 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 > > > > #include "msm_drv.h" > > @@ -1463,6 +1464,10 @@ void msm_dp_debugfs_init(struct msm_dp > > *dp_display, struct drm_minor *minor) > > dp = container_of(dp_display, struct dp_display_private, dp_display); > > dev = &dp->pdev->dev; > > > > + /* Only create one set of debugfs per DP instance */ > > + if (minor->type != DRM_MINOR_PRIMARY) > > + return; > > + > > dp->debug = dp_debug_get(dev, dp->panel, dp->usbpd, > > dp->link, dp->dp_display.connector, > > minor);
Re: [Freedreno] [PATCH] drm/msm/dp: Move debugfs files into subdirectory
On 2021-10-15 16:17, Bjorn Andersson wrote:
In the cleanup path of the MSM DP driver the DP driver's debugfs files
are destroyed by invoking debugfs_remove_recursive() on debug->root,
which during initialization has been set to minor->debugfs_root.
To allow cleaning up the DP driver's debugfs files either each dentry
needs to be kept track of or the files needs to be put in a
subdirectory
which can be removed in one go.
By choosing to put the debugfs files in a subdirectory, based on the
name of the associated connector this also solves the problem that
these
names would collide as support for multiple DP instances are
introduced.
One alternative solution to the problem with colliding file names would
have been to put keep track of the individual files and put them under
the connector's debugfs directory. But while the drm_connector has been
allocated, its associated debugfs directory has not been created at the
time of initialization of the dp_debug.
Signed-off-by: Bjorn Andersson
I have been thinking about this problem ever since multi-DP has been
posted :)
Creating sub-directories seems right but at the moment it looks like IGT
which
uses these debugfs nodes doesnt check sub-directories:
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c#L215
It looks for the DP debugfs nodes under /sys/kernel/debug/dri/*/
We have to fix IGT too to be able to handle multi-DP cases. I will try
to come up
with a proposal to address this.
Till then, can we go with the other solution to keep track of the
dentries?
---
This depends on
https://lore.kernel.org/linux-arm-msm/20211010030435.4000642-1-bjorn.anders...@linaro.org/
reducing the connector from a double pointer.
drivers/gpu/drm/msm/dp/dp_debug.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index da4323556ef3..67da4c69eca1 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -210,26 +210,29 @@ static const struct file_operations
test_active_fops = {
static int dp_debug_init(struct dp_debug *dp_debug, struct drm_minor
*minor)
{
int rc = 0;
+ char path[64];
struct dp_debug_private *debug = container_of(dp_debug,
struct dp_debug_private, dp_debug);
- debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
+ snprintf(path, sizeof(path), "msm_dp-%s", debug->connector->name);
+
+ debug->root = debugfs_create_dir(path, minor->debugfs_root);
+
+ debugfs_create_file("dp_debug", 0444, debug->root,
debug, &dp_debug_fops);
debugfs_create_file("msm_dp_test_active", 0444,
- minor->debugfs_root,
+ debug->root,
debug, &test_active_fops);
debugfs_create_file("msm_dp_test_data", 0444,
- minor->debugfs_root,
+ debug->root,
debug, &dp_test_data_fops);
debugfs_create_file("msm_dp_test_type", 0444,
- minor->debugfs_root,
+ debug->root,
debug, &dp_test_type_fops);
- debug->root = minor->debugfs_root;
-
return rc;
}
Re: [Freedreno] [PATCH] drm/msm/dp: Only create debugfs for PRIMARY minor
On 2021-10-15 16:13, Bjorn Andersson wrote: dpu_kms_debugfs_init() and hence dp_debug_get() gets invoked for each minor being registered. But dp_debug_get() will allocate a new struct dp_debug for each call and this will be associated as dp->debug. As such dp_debug will create debugfs files in both the PRIMARY and the RENDER minor's debugfs directory, but only the last reference will be remembered. The only use of this reference today is in the cleanup path in dp_display_deinit_sub_modules() and the dp_debug_private object does outlive the debugfs entries in either case, so there doesn't seem to be any adverse effects of this, but per the code the current behavior is unexpected, so change it to only create dp_debug for the PRIMARY minor. If i understand correctly, today because of this, we get redundant debugfs nodes right? /sys/kernel/debug/dri//dp_debug /sys/kernel/debug/dri//dp_debug Both of these will hold the same information as they are for the same DP controller right? In that case, this is true even for the other DPU kms information too. Why not move this check one level up to dpu_kms_debugfs_init? Signed-off-by: Bjorn Andersson --- drivers/gpu/drm/msm/dp/dp_display.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 3aa67c53dbc0..06773b58bb60 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 #include "msm_drv.h" @@ -1463,6 +1464,10 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor) dp = container_of(dp_display, struct dp_display_private, dp_display); dev = &dp->pdev->dev; + /* Only create one set of debugfs per DP instance */ + if (minor->type != DRM_MINOR_PRIMARY) + return; + dp->debug = dp_debug_get(dev, dp->panel, dp->usbpd, dp->link, dp->dp_display.connector, minor);
[Freedreno] [PATCH v2] drm/msm/dp: Use the connector passed to dp_debug_get()
The debugfs code is provided an array of a single drm_connector. Then to
access the connector, the list of all connectors of the DRM device is
traversed and all non-DisplayPort connectors are skipped, to find the
one and only DisplayPort connector.
But as we move to support multiple DisplayPort controllers this will now
find multiple connectors and has no way to distinguish them.
Pass the single connector to dp_debug_get() and use this in the debugfs
functions instead, both to simplify the code and the support the
multiple instances.
Reviewed-by: Abhinav Kumar
Reviewed-by: Stephen Boyd
Signed-off-by: Bjorn Andersson
---
Changes since v1:
- Reduce the indirection in the !DEBUG_FS stub function in dp_debug.h as well
drivers/gpu/drm/msm/dp/dp_debug.c | 131 ++--
drivers/gpu/drm/msm/dp/dp_debug.h | 4 +-
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
3 files changed, 47 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index af709d93bb9f..da4323556ef3 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -24,7 +24,7 @@ struct dp_debug_private {
struct dp_usbpd *usbpd;
struct dp_link *link;
struct dp_panel *panel;
- struct drm_connector **connector;
+ struct drm_connector *connector;
struct device *dev;
struct drm_device *drm_dev;
@@ -97,59 +97,35 @@ DEFINE_SHOW_ATTRIBUTE(dp_debug);
static int dp_test_data_show(struct seq_file *m, void *data)
{
- struct drm_device *dev;
- struct dp_debug_private *debug;
- struct drm_connector *connector;
- struct drm_connector_list_iter conn_iter;
+ const struct dp_debug_private *debug = m->private;
+ const struct drm_connector *connector = debug->connector;
u32 bpc;
- debug = m->private;
- dev = debug->drm_dev;
- drm_connector_list_iter_begin(dev, &conn_iter);
- drm_for_each_connector_iter(connector, &conn_iter) {
-
- if (connector->connector_type !=
- DRM_MODE_CONNECTOR_DisplayPort)
- continue;
-
- if (connector->status == connector_status_connected) {
- bpc = debug->link->test_video.test_bit_depth;
- seq_printf(m, "hdisplay: %d\n",
- debug->link->test_video.test_h_width);
- seq_printf(m, "vdisplay: %d\n",
- debug->link->test_video.test_v_height);
- seq_printf(m, "bpc: %u\n",
- dp_link_bit_depth_to_bpc(bpc));
- } else
- seq_puts(m, "0");
+ if (connector->status == connector_status_connected) {
+ bpc = debug->link->test_video.test_bit_depth;
+ seq_printf(m, "hdisplay: %d\n",
+ debug->link->test_video.test_h_width);
+ seq_printf(m, "vdisplay: %d\n",
+ debug->link->test_video.test_v_height);
+ seq_printf(m, "bpc: %u\n",
+ dp_link_bit_depth_to_bpc(bpc));
+ } else {
+ seq_puts(m, "0");
}
- drm_connector_list_iter_end(&conn_iter);
-
return 0;
}
DEFINE_SHOW_ATTRIBUTE(dp_test_data);
static int dp_test_type_show(struct seq_file *m, void *data)
{
- struct dp_debug_private *debug = m->private;
- struct drm_device *dev = debug->drm_dev;
- struct drm_connector *connector;
- struct drm_connector_list_iter conn_iter;
-
- drm_connector_list_iter_begin(dev, &conn_iter);
- drm_for_each_connector_iter(connector, &conn_iter) {
-
- if (connector->connector_type !=
- DRM_MODE_CONNECTOR_DisplayPort)
- continue;
+ const struct dp_debug_private *debug = m->private;
+ const struct drm_connector *connector = debug->connector;
- if (connector->status == connector_status_connected)
- seq_printf(m, "%02x", DP_TEST_LINK_VIDEO_PATTERN);
- else
- seq_puts(m, "0");
- }
- drm_connector_list_iter_end(&conn_iter);
+ if (connector->status == connector_status_connected)
+ seq_printf(m, "%02x", DP_TEST_LINK_VIDEO_PATTERN);
+ else
+ seq_puts(m, "0");
return 0;
}
@@ -161,14 +137,12 @@ static ssize_t dp_test_active_write(struct file *file,
{
char *input_buffer;
int status = 0;
- struct dp_debug_private *debug;
- struct drm_device *dev;
- struct drm_connector *connector;
- struct drm_connector_list_iter conn_iter;
+ const struct dp_debug_private *debug;
+ const struct drm_connector *connector;
int val = 0;
debug = ((struct seq_file *)file->privat
[Freedreno] [PATCH] drm/msm/dp: Move debugfs files into subdirectory
In the cleanup path of the MSM DP driver the DP driver's debugfs files
are destroyed by invoking debugfs_remove_recursive() on debug->root,
which during initialization has been set to minor->debugfs_root.
To allow cleaning up the DP driver's debugfs files either each dentry
needs to be kept track of or the files needs to be put in a subdirectory
which can be removed in one go.
By choosing to put the debugfs files in a subdirectory, based on the
name of the associated connector this also solves the problem that these
names would collide as support for multiple DP instances are introduced.
One alternative solution to the problem with colliding file names would
have been to put keep track of the individual files and put them under
the connector's debugfs directory. But while the drm_connector has been
allocated, its associated debugfs directory has not been created at the
time of initialization of the dp_debug.
Signed-off-by: Bjorn Andersson
---
This depends on
https://lore.kernel.org/linux-arm-msm/20211010030435.4000642-1-bjorn.anders...@linaro.org/
reducing the connector from a double pointer.
drivers/gpu/drm/msm/dp/dp_debug.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c
b/drivers/gpu/drm/msm/dp/dp_debug.c
index da4323556ef3..67da4c69eca1 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -210,26 +210,29 @@ static const struct file_operations test_active_fops = {
static int dp_debug_init(struct dp_debug *dp_debug, struct drm_minor *minor)
{
int rc = 0;
+ char path[64];
struct dp_debug_private *debug = container_of(dp_debug,
struct dp_debug_private, dp_debug);
- debugfs_create_file("dp_debug", 0444, minor->debugfs_root,
+ snprintf(path, sizeof(path), "msm_dp-%s", debug->connector->name);
+
+ debug->root = debugfs_create_dir(path, minor->debugfs_root);
+
+ debugfs_create_file("dp_debug", 0444, debug->root,
debug, &dp_debug_fops);
debugfs_create_file("msm_dp_test_active", 0444,
- minor->debugfs_root,
+ debug->root,
debug, &test_active_fops);
debugfs_create_file("msm_dp_test_data", 0444,
- minor->debugfs_root,
+ debug->root,
debug, &dp_test_data_fops);
debugfs_create_file("msm_dp_test_type", 0444,
- minor->debugfs_root,
+ debug->root,
debug, &dp_test_type_fops);
- debug->root = minor->debugfs_root;
-
return rc;
}
--
2.29.2
[Freedreno] [PATCH] drm/msm/dp: Only create debugfs for PRIMARY minor
dpu_kms_debugfs_init() and hence dp_debug_get() gets invoked for each minor being registered. But dp_debug_get() will allocate a new struct dp_debug for each call and this will be associated as dp->debug. As such dp_debug will create debugfs files in both the PRIMARY and the RENDER minor's debugfs directory, but only the last reference will be remembered. The only use of this reference today is in the cleanup path in dp_display_deinit_sub_modules() and the dp_debug_private object does outlive the debugfs entries in either case, so there doesn't seem to be any adverse effects of this, but per the code the current behavior is unexpected, so change it to only create dp_debug for the PRIMARY minor. Signed-off-by: Bjorn Andersson --- drivers/gpu/drm/msm/dp/dp_display.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 3aa67c53dbc0..06773b58bb60 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 #include "msm_drv.h" @@ -1463,6 +1464,10 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor) dp = container_of(dp_display, struct dp_display_private, dp_display); dev = &dp->pdev->dev; + /* Only create one set of debugfs per DP instance */ + if (minor->type != DRM_MINOR_PRIMARY) + return; + dp->debug = dp_debug_get(dev, dp->panel, dp->usbpd, dp->link, dp->dp_display.connector, minor); -- 2.29.2
Re: [Freedreno] [PATCH 2/2] drm/msm/hdmi: switch to drm_bridge_connector
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.
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(&hdmi->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 @@
*/
#include
+#include
+#include "msm_kms.h"
#include "hdmi.h"
-struct hdmi_bridge {
- struct drm_bridge base;
- struct hdmi *hdmi;
-};
-#define to_hdmi_bridge(x) container_of(x, struct hdmi_bridge, base)
-
void msm_hdmi_bridge_destroy(struct drm_bridge *bridge)
{
+ struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
+
+ msm_hdmi_hpd_disable(hdmi_bridge);
}
static void msm_hdmi_power_on(struct drm_bridge *bridge)
@@ -251,14 +250,76 @@ static void msm_hdmi_bridge_mode_set(struct
drm_
Re: [Freedreno] [PATCH 1/2] drm/msm/hdmi: use bulk regulator API
On 2021-10-14 17:10, Dmitry Baryshkov wrote:
Switch to using bulk regulator API instead of hand coding loops.
Nice cleanup!
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 34 +++
drivers/gpu/drm/msm/hdmi/hdmi.h | 6 ++--
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c| 20 -
drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 24 ++--
drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 33 --
5 files changed, 40 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c
b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 737453b6e596..db17a000d968 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -154,19 +154,13 @@ static struct hdmi *msm_hdmi_init(struct
platform_device *pdev)
ret = -ENOMEM;
goto fail;
}
- for (i = 0; i < config->hpd_reg_cnt; i++) {
- struct regulator *reg;
-
- reg = devm_regulator_get(&pdev->dev,
- config->hpd_reg_names[i]);
- if (IS_ERR(reg)) {
- ret = PTR_ERR(reg);
- DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %s
(%d)\n",
- config->hpd_reg_names[i], ret);
- goto fail;
- }
+ for (i = 0; i < config->hpd_reg_cnt; i++)
+ hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
- hdmi->hpd_regs[i] = reg;
+ ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt,
hdmi->hpd_regs);
+ if (ret) {
+ DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %d\n",
ret);
+ goto fail;
}
hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
@@ -177,19 +171,11 @@ static struct hdmi *msm_hdmi_init(struct
platform_device *pdev)
ret = -ENOMEM;
goto fail;
}
- for (i = 0; i < config->pwr_reg_cnt; i++) {
- struct regulator *reg;
- reg = devm_regulator_get(&pdev->dev,
- config->pwr_reg_names[i]);
- if (IS_ERR(reg)) {
- ret = PTR_ERR(reg);
- DRM_DEV_ERROR(&pdev->dev, "failed to get pwr regulator: %s
(%d)\n",
- config->pwr_reg_names[i], ret);
- goto fail;
- }
-
- hdmi->pwr_regs[i] = reg;
+ ret = devm_regulator_bulk_get(&pdev->dev, config->pwr_reg_cnt,
hdmi->pwr_regs);
+ if (ret) {
+ DRM_DEV_ERROR(&pdev->dev, "failed to get pwr regulator: %d\n",
ret);
+ goto fail;
}
hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h
b/drivers/gpu/drm/msm/hdmi/hdmi.h
index d0b84f0abee1..82261078c6b1 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -56,8 +56,8 @@ struct hdmi {
void __iomem *qfprom_mmio;
phys_addr_t mmio_phy_addr;
- struct regulator **hpd_regs;
- struct regulator **pwr_regs;
+ struct regulator_bulk_data *hpd_regs;
+ struct regulator_bulk_data *pwr_regs;
struct clk **hpd_clks;
struct clk **pwr_clks;
@@ -163,7 +163,7 @@ struct hdmi_phy {
void __iomem *mmio;
struct hdmi_phy_cfg *cfg;
const struct hdmi_phy_funcs *funcs;
- struct regulator **regs;
+ struct regulator_bulk_data *regs;
struct clk **clks;
};
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 6e380db9287b..f04eb4a70f0d 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -28,13 +28,9 @@ static void msm_hdmi_power_on(struct drm_bridge
*bridge)
pm_runtime_get_sync(&hdmi->pdev->dev);
- for (i = 0; i < config->pwr_reg_cnt; i++) {
- ret = regulator_enable(hdmi->pwr_regs[i]);
- if (ret) {
- DRM_DEV_ERROR(dev->dev, "failed to enable pwr regulator: %s
(%d)\n",
- config->pwr_reg_names[i], ret);
- }
- }
+ ret = regulator_bulk_enable(config->pwr_reg_cnt, hdmi->pwr_regs);
+ if (ret)
+ DRM_DEV_ERROR(dev->dev, "failed to enable pwr regulator: %d\n",
ret);
if (config->pwr_clk_cnt > 0) {
DBG("pixclock: %lu", hdmi->pixclock);
@@ -70,13 +66,9 @@ static void power_off(struct drm_bridge *bridge)
for (i = 0; i < config->pwr_clk_cnt; i++)
clk_disable_unprepare(hdmi->pwr_clks[i]);
- for (i = 0; i < config->pwr_reg_cnt; i++) {
- ret = regulator_disable(hdmi->pwr_regs[i]);
- if (ret) {
- DRM_DEV_ERROR(dev->dev, "failed to disable pwr regulator: %s
(%d)\n",
- config->pwr_reg_
Re: [Freedreno] [PATCH] ARM: dts: qcom-apq8064: stop using legacy clock names for HDMI
Quoting Dmitry Baryshkov (2021-10-14 14:42:21) > Stop using legacy clock names (with _clk suffix) for HDMI and HDMI PHY > device tree nodes. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [Freedreno] [PATCH 0/3] drm/msm: drop old eDP support
Hi Dmitry No concerns from our side to drop this. Reviewed-by: Abhinav Kumar for the series. Thanks Abhinav On 2021-10-01 09:50, Dmitry Baryshkov wrote: MSM DRM driver has support for eDP block present on MSM 8x74/8x84 SoC families. However since addition back in 2015 this driver received only generic fixes. No actual devices with these SoCs supported upstream (or by the community) seem to support eDP panels. Judging from downstream kernels the eDP was present only on MSM8974 LIQUID or on APQ8084 CDP. Remove this driver. Dmitry Baryshkov (3): drm/msm/mdp5: drop eDP support drm/msm/edp: drop old eDP support dt-bindings: display/msm: remove edp.txt .../devicetree/bindings/display/msm/edp.txt| 56 - drivers/gpu/drm/msm/Makefile |6 - drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 17 +- drivers/gpu/drm/msm/edp/edp.c | 198 --- drivers/gpu/drm/msm/edp/edp.h | 77 -- drivers/gpu/drm/msm/edp/edp.xml.h | 388 -- drivers/gpu/drm/msm/edp/edp_aux.c | 265 drivers/gpu/drm/msm/edp/edp_bridge.c | 111 -- drivers/gpu/drm/msm/edp/edp_connector.c| 132 -- drivers/gpu/drm/msm/edp/edp_ctrl.c | 1375 drivers/gpu/drm/msm/edp/edp_phy.c | 98 -- drivers/gpu/drm/msm/msm_drv.c |2 - drivers/gpu/drm/msm/msm_drv.h | 12 - 13 files changed, 1 insertion(+), 2736 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/msm/edp.txt delete mode 100644 drivers/gpu/drm/msm/edp/edp.c delete mode 100644 drivers/gpu/drm/msm/edp/edp.h delete mode 100644 drivers/gpu/drm/msm/edp/edp.xml.h delete mode 100644 drivers/gpu/drm/msm/edp/edp_aux.c delete mode 100644 drivers/gpu/drm/msm/edp/edp_bridge.c delete mode 100644 drivers/gpu/drm/msm/edp/edp_connector.c delete mode 100644 drivers/gpu/drm/msm/edp/edp_ctrl.c delete mode 100644 drivers/gpu/drm/msm/edp/edp_phy.c
Re: [Freedreno] [Intel-gfx] [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 &drm_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().
But maybe that's not so easy. Would still need to bail out
from drm_add_edid_modes() I guess.
--
Ville Syrjälä
Intel
Re: [Freedreno] [PATCH 1/2] drm/msm/hdmi: use bulk regulator API
Quoting Dmitry Baryshkov (2021-10-14 17:10:59) > Switch to using bulk regulator API instead of hand coding loops. > > Signed-off-by: Dmitry Baryshkov > --- Reviewed-by: Stephen Boyd
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
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 &drm_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().
--
Ville Syrjälä
Intel
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
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 &drm_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 ?
BR
Claudio Suarez
Re: [Freedreno] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver
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
Harry
> Signed-off-by: Claudio Suarez
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 +++
> .../gpu/drm/amd/amdgpu/amdgpu_connectors.h| 2 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 4 +-
> .../gpu/drm/amd/amdgpu/atombios_encoders.c| 6 +--
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-
> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 ++-
> drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
> drivers/gpu/drm/amd/display/dc/dm_helpers.h | 2 +-
> 9 files changed, 39 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index b9c11c2b2885..7b41a1120b70 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -25,6 +25,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -108,7 +109,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
> *connector)
> case DRM_MODE_CONNECTOR_DVII:
> case DRM_MODE_CONNECTOR_HDMIB:
> if (amdgpu_connector->use_digital) {
> - if
> (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
> + if (amdgpu_connector_is_hdmi_monitor(connector)) {
> if (connector->display_info.bpc)
> bpc = connector->display_info.bpc;
> }
> @@ -116,7 +117,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
> *connector)
> break;
> case DRM_MODE_CONNECTOR_DVID:
> case DRM_MODE_CONNECTOR_HDMIA:
> - if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
> + if (amdgpu_connector_is_hdmi_monitor(connector)) {
> if (connector->display_info.bpc)
> bpc = connector->display_info.bpc;
> }
> @@ -125,7 +126,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
> *connector)
> dig_connector = amdgpu_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(amdgpu_connector_edid(connector))) {
> + (amdgpu_connector_is_hdmi_monitor(connector))) {
> if (connector->display_info.bpc)
> bpc = connector->display_info.bpc;
> }
> @@ -149,7 +150,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
> *connector)
> break;
> }
>
> - if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
> + if (amdgpu_connector_is_hdmi_monitor(connector)) {
> /*
>* Pre DCE-8 hw can't handle > 12 bpc, and more than 12 bpc
> doesn't make
>* much sense without support for > 12 bpc framebuffers. RGB
> 4:4:4 at
> @@ -315,8 +316,10 @@ static void amdgpu_connector_get_edid(struct
> drm_connector *connector)
> if (!amdgpu_connector->edid) {
> /* some laptops provide a hardcoded edid in rom for LCDs */
> if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
> - (connector->connector_type == DRM_MODE_CONNECTOR_eDP)))
> + (connector->connector_type == DRM_MODE_CONNECTOR_eDP))) {
> amdgp
Re: [Freedreno] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 15, 2021 at 06:18:34PM +0300, Jani Nikula wrote:
> On Fri, 15 Oct 2021, Ville Syrjälä wrote:
> > On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
> >> On Fri, 15 Oct 2021, Claudio Suarez wrote:
> >> > 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_connector.c | 5 +
> >> > drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> >> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> >> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> >> > 4 files changed, 9 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> >> > b/drivers/gpu/drm/i915/display/intel_connector.c
> >> > index 9bed1ccecea0..3346b55df6e1 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> >> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector
> >> > *connector,
> >> > return ret;
> >> > }
> >> >
> >> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> >> > +{
> >> > +return connector->display_info.is_hdmi;
> >> > +}
> >> > +
> >>
> >> A helper like this belongs in drm, not i915. Seems useful in other
> >> drivers too.
> >
> > Not sure it's actually helpful for i915. We end up having to root around
> > in the display_info in a lot of places anyway. So a helper for single
> > boolean seems a bit out of place perhaps.
>
> *shrug*
>
> Maybe it's just my frustration at the lack of interfaces and poking
> around in the depths of nested structs and pointer chasing that's coming
> through. You just need to change so many things if you want to later
> refactor where "is hdmi" comes from and is stored.
>
> Anyway, if a helper is being added like in this series, I think it
> should be one helper in drm, not redundant copies in multiple
> drivers. Or we should not have the helper(s) at all. One or the other,
> not the worst of both worlds.
Thank you all for your comments :)
The big work here was to figure out which drm_detect_hdmi_monitor() can be
replaced. Changing a helper isn't a problem.
I'll send a new patch in a few hours.
BR
Claudio Suarez.
Re: [Freedreno] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 15, 2021 at 03:30:49PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 01:37:13PM +0200, Claudio Suarez wrote:
> > 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_connector.c | 5 +
> > drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> > 4 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> > b/drivers/gpu/drm/i915/display/intel_connector.c
> > index 9bed1ccecea0..3346b55df6e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector
> > *connector,
> > return ret;
> > }
> >
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> > +{
> > + return connector->display_info.is_hdmi;
> > +}
> > +
> > static const struct drm_prop_enum_list force_audio_names[] = {
> > { HDMI_AUDIO_OFF_DVI, "force-dvi" },
> > { HDMI_AUDIO_OFF, "off" },
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
> > b/drivers/gpu/drm/i915/display/intel_connector.h
> > index 661a37a3c6d8..ceda6e72ece6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.h
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> > @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector
> > *connector);
> > int intel_connector_update_modes(struct drm_connector *connector,
> > struct edid *edid);
> > int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter
> > *adapter);
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
> > void intel_attach_force_audio_property(struct drm_connector *connector);
> > void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> > void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index b04685bb6439..2b1d7c5bebdd 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 =
> > intel_connector_is_hdmi_monitor(connector);
>
> Hmm. Have we parse the EDID by this point actually? I don't think that
> was the case in the past but maybe it changed at some point.
Yes, I think so. The complete code is:
edid = drm_get_edid(connector, i2c);
if (!edid && !intel_gmbus_is_forced_bit(i2c)) {
drm_dbg_kms(&dev_priv->drm,
"HDMI GMBUS EDID read failed, retry using GPIO
bit-banging\n");
intel_gmbus_force_bit(i2c, true);
edid = drm_get_edid(connector, i2c);
intel_gmbus_force_bit(i2c, false);
}
intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
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 =
intel_connector_is_hdmi_monitor(connector);
The edid value comes from drm_get_edid(), first or second.
drm_get_edid() internally calls drm_connector_update_edid_property()
drm_connector_update_edid_property() calls drm_add_display_info() and parses
the edid.
So, the edid is parsed.
I checked this and I read the docs many times because at the first time I felt
something
was wrong. But that is the sequence of calls.
>
> >
> > connected = true;
> > }
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index 6cb27599ea03..a32279e4fee8 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;
> >
Re: [Freedreno] [PATCH 0/9] treewide: simplify getting .driver_data
On Mon, 20 Sep 2021 11:05:12 +0200, Wolfram Sang wrote: > I got tired of fixing this in Renesas drivers manually, so I took the big > hammer. Remove this cumbersome code pattern which got copy-pasted too much > already: > > - struct platform_device *pdev = to_platform_device(dev); > - struct ep93xx_keypad *keypad = platform_get_drvdata(pdev); > + struct ep93xx_keypad *keypad = dev_get_drvdata(dev); > > [...] Applied, thanks! [1/9] dmaengine: stm32-dmamux: simplify getting .driver_data (no commit info) [2/9] firmware: meson: simplify getting .driver_data (no commit info) [3/9] gpio: xilinx: simplify getting .driver_data (no commit info) [4/9] drm/msm: simplify getting .driver_data (no commit info) [5/9] drm/panfrost: simplify getting .driver_data (no commit info) [6/9] iio: common: cros_ec_sensors: simplify getting .driver_data (no commit info) [7/9] net: mdio: mdio-bcm-iproc: simplify getting .driver_data (no commit info) [8/9] platform: chrome: cros_ec_sensorhub: simplify getting .driver_data (no commit info) [9/9] remoteproc: omap_remoteproc: simplify getting .driver_data commit: c34bfafd7c6ce8bdb5205aa990973b6ec7a6557c Best regards, -- Bjorn Andersson
Re: [Freedreno] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, 15 Oct 2021, Ville Syrjälä wrote:
> On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
>> On Fri, 15 Oct 2021, Claudio Suarez wrote:
>> > 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_connector.c | 5 +
>> > drivers/gpu/drm/i915/display/intel_connector.h | 1 +
>> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
>> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
>> > 4 files changed, 9 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
>> > b/drivers/gpu/drm/i915/display/intel_connector.c
>> > index 9bed1ccecea0..3346b55df6e1 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
>> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector
>> > *connector,
>> >return ret;
>> > }
>> >
>> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
>> > +{
>> > + return connector->display_info.is_hdmi;
>> > +}
>> > +
>>
>> A helper like this belongs in drm, not i915. Seems useful in other
>> drivers too.
>
> Not sure it's actually helpful for i915. We end up having to root around
> in the display_info in a lot of places anyway. So a helper for single
> boolean seems a bit out of place perhaps.
*shrug*
Maybe it's just my frustration at the lack of interfaces and poking
around in the depths of nested structs and pointer chasing that's coming
through. You just need to change so many things if you want to later
refactor where "is hdmi" comes from and is stored.
Anyway, if a helper is being added like in this series, I think it
should be one helper in drm, not redundant copies in multiple
drivers. Or we should not have the helper(s) at all. One or the other,
not the worst of both worlds.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
[Freedreno] [PATCH 00/15] replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Copy&paste from the TODO document 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. = 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/amd/amdgpu/amdgpu_connectors.c| 23 --- drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.h| 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 4 ++-- drivers/gpu/drm/amd/amdgpu/atombios_encoders.c| 6 +++--- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 --- drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +- drivers/gpu/drm/amd/display/dc/dm_helpers.h | 2 +- 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| 2 ++ 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_connector.c| 5 + drivers/gpu/drm/i915/display/intel_connector.h| 1 + 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/nouveau/nouveau_connector.c | 2 +- drivers/gpu/drm/nouveau/nouveau_connector.h | 6 ++ drivers/gpu/drm/radeon/atombios_encoders.c| 6 +++--- drivers/gpu/drm/radeon/radeon_connectors.c| 20 ++-- drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++-- drivers/gpu/drm/radeon/radeon_mode.h | 1 + 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 +++--- drivers/gpu/drm/zte/zx_hdmi.c | 4 ++-- 37 files changed, 112 insertions(+), 96 deletions(-) Best regards. Claudio Suarez
[Freedreno] [PATCH 06/15] 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, &gma_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
[Freedreno] [PATCH 13/15] 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
[Freedreno] [PATCH 14/15] 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 +---
drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_connector.h | 6 ++
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index d7b9f7f8c9e3..fadd58b015d6 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 (nouveau_connector_is_hdmi_monitor(&nv_connector->base))
return;
hdmi = &nv_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))
+
!nouveau_connector_is_hdmi_monitor(&nv_connector->base))
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..0a138bfb8f32 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 = &asyh->state.adjusted_mode;
struct drm_display_mode *umode = &asyh->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 {
+nouveau_connector_is_hdmi_monitor(connector {
u32 bX = asyc->scaler.underscan.hborder;
u32 bY = asyc->scaler.underscan.vborder;
u32 r = (asyh->view.oH << 19) / asyh->view.oW;
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 22b83a6577eb..211543373b72 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1010,7 +1010,7 @@ get_tmds_link_bandwidth(struct drm_connector *connector)
unsigned duallink_scale =
nouveau_duallink && nv_encoder->dcb->duallink_possible ? 2 : 1;
- if (drm_detect_hdmi_monitor(nv_connector->edid)) {
+ if (nouveau_connector_is_hdmi_monitor(connector)) {
info = &nv_connector->base.display_info;
duallink_scale = 1;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h
b/drivers/gpu/drm/nouveau/nouveau_connector.h
index 40f90e353540..299f3a3b2331 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.h
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
@@ -167,6 +167,12 @@ nouveau_connector_is_mst(struct drm_connector *connector)
return encoder->encoder_type == DRM_MODE_ENCODER_DPMST;
}
+static inline bool
+nouveau_connector_is_hdmi_monitor(struct drm_connector *connector)
+{
+ return connector->display_info.is_hdmi;
+}
+
#define nouveau_for_each_non_mst_connector_iter(connector, iter) \
drm_for_each_connector_iter(connector, iter) \
for_each_if(!nouveau_connector_is_mst(connector))
--
2.33.0
[Freedreno] [PATCH 04/15] 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 | 20 ++--
drivers/gpu/drm/radeon/radeon_display.c| 2 +-
drivers/gpu/drm/radeon/radeon_encoders.c | 4 ++--
drivers/gpu/drm/radeon/radeon_mode.h | 1 +
5 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c
b/drivers/gpu/drm/radeon/atombios_encoders.c
index 0fce73b9a646..29a140732f71 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 (radeon_connector_is_hdmi_monitor(connector) &&
(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 (radeon_connector_is_hdmi_monitor(connector) &&
(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 (radeon_connector_is_hdmi_monitor(connector) &&
(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..0200f094467c 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 (radeon_connector_is_hdmi_monitor(connector)) {
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 (radeon_connector_is_hdmi_monitor(connector)) {
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
[Freedreno] [PATCH 09/15] 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
[Freedreno] [PATCH 07/15] 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
[Freedreno] [PATCH 03/15] 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
[Freedreno] [PATCH 10/15] 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
[Freedreno] [PATCH 12/15] 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
[Freedreno] [PATCH 15/15] 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_connector.c | 5 +
drivers/gpu/drm/i915/display/intel_connector.h | 1 +
drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
b/drivers/gpu/drm/i915/display/intel_connector.c
index 9bed1ccecea0..3346b55df6e1 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.c
+++ b/drivers/gpu/drm/i915/display/intel_connector.c
@@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
return ret;
}
+bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
+{
+ return connector->display_info.is_hdmi;
+}
+
static const struct drm_prop_enum_list force_audio_names[] = {
{ HDMI_AUDIO_OFF_DVI, "force-dvi" },
{ HDMI_AUDIO_OFF, "off" },
diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
b/drivers/gpu/drm/i915/display/intel_connector.h
index 661a37a3c6d8..ceda6e72ece6 100644
--- a/drivers/gpu/drm/i915/display/intel_connector.h
+++ b/drivers/gpu/drm/i915/display/intel_connector.h
@@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector
*connector);
int intel_connector_update_modes(struct drm_connector *connector,
struct edid *edid);
int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
+bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
void intel_attach_force_audio_property(struct drm_connector *connector);
void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
void intel_attach_aspect_ratio_property(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..2b1d7c5bebdd 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 =
intel_connector_is_hdmi_monitor(connector);
connected = true;
}
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6cb27599ea03..a32279e4fee8 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 =
+
intel_connector_is_hdmi_monitor(connector);
}
} else
status = connector_status_disconnected;
--
2.33.0
[Freedreno] [PATCH 01/15] 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 &drm_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);
drm_warn(connector->dev, "%s: EDID invalid.\n",
connector->name);
return 0;
--
2.33.0
[Freedreno] [PATCH 11/15] drm/zte: 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/zte/zx_hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c index cd79ca0a92a9..7df682d90723 100644 --- a/drivers/gpu/drm/zte/zx_hdmi.c +++ b/drivers/gpu/drm/zte/zx_hdmi.c @@ -265,9 +265,9 @@ static int zx_hdmi_connector_get_modes(struct drm_connector *connector) if (!edid) return 0; - hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); - hdmi->sink_has_audio = drm_detect_monitor_audio(edid); drm_connector_update_edid_property(connector, edid); + hdmi->sink_is_hdmi = connector->display_info.is_hdmi; + hdmi->sink_has_audio = drm_detect_monitor_audio(edid); ret = drm_add_edid_modes(connector, edid); kfree(edid); -- 2.33.0
[Freedreno] [PATCH 05/15] 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
[Freedreno] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver
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()
Signed-off-by: Claudio Suarez
---
.../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 23 +++
.../gpu/drm/amd/amdgpu/amdgpu_connectors.h| 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_encoders.c | 4 +-
.../gpu/drm/amd/amdgpu/atombios_encoders.c| 6 +--
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 39 ++-
drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
drivers/gpu/drm/amd/display/dc/dm_helpers.h | 2 +-
9 files changed, 39 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index b9c11c2b2885..7b41a1120b70 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -25,6 +25,7 @@
*/
#include
+#include
#include
#include
#include
@@ -108,7 +109,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
case DRM_MODE_CONNECTOR_DVII:
case DRM_MODE_CONNECTOR_HDMIB:
if (amdgpu_connector->use_digital) {
- if
(drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ if (amdgpu_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -116,7 +117,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
break;
case DRM_MODE_CONNECTOR_DVID:
case DRM_MODE_CONNECTOR_HDMIA:
- if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ if (amdgpu_connector_is_hdmi_monitor(connector)) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -125,7 +126,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
dig_connector = amdgpu_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(amdgpu_connector_edid(connector))) {
+ (amdgpu_connector_is_hdmi_monitor(connector))) {
if (connector->display_info.bpc)
bpc = connector->display_info.bpc;
}
@@ -149,7 +150,7 @@ int amdgpu_connector_get_monitor_bpc(struct drm_connector
*connector)
break;
}
- if (drm_detect_hdmi_monitor(amdgpu_connector_edid(connector))) {
+ if (amdgpu_connector_is_hdmi_monitor(connector)) {
/*
* Pre DCE-8 hw can't handle > 12 bpc, and more than 12 bpc
doesn't make
* much sense without support for > 12 bpc framebuffers. RGB
4:4:4 at
@@ -315,8 +316,10 @@ static void amdgpu_connector_get_edid(struct drm_connector
*connector)
if (!amdgpu_connector->edid) {
/* some laptops provide a hardcoded edid in rom for LCDs */
if (((connector->connector_type == DRM_MODE_CONNECTOR_LVDS) ||
-(connector->connector_type == DRM_MODE_CONNECTOR_eDP)))
+(connector->connector_type == DRM_MODE_CONNECTOR_eDP))) {
amdgpu_connector->edid =
amdgpu_connector_get_hardcoded_edid(adev);
+ drm_connector_update_edid_property(connector,
amdgpu_connector->edid);
+ }
}
}
@@ -326,6 +329,7 @@ static void amdgpu_connector_free_edid(struct drm_connector
*connector)
kfree(amdgpu_connector->edid);
amdgpu_connector->edid = NULL;
+ drm_connector_update_edid_property(connector, NULL);
}
static int amdgpu_connector_ddc_get_modes(struct drm_connector *connector)
@@ -1170,7 +1174,7 @@ static enum drm_mode_status
amdgpu_connector_dvi_mode_valid(struct drm_connector
[Freedreno] [PATCH 08/15] 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
Re: [Freedreno] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 15, 2021 at 03:44:48PM +0300, Jani Nikula wrote:
> On Fri, 15 Oct 2021, Claudio Suarez wrote:
> > 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_connector.c | 5 +
> > drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> > 4 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> > b/drivers/gpu/drm/i915/display/intel_connector.c
> > index 9bed1ccecea0..3346b55df6e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_connector.c
> > +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> > @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector
> > *connector,
> > return ret;
> > }
> >
> > +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> > +{
> > + return connector->display_info.is_hdmi;
> > +}
> > +
>
> A helper like this belongs in drm, not i915. Seems useful in other
> drivers too.
Not sure it's actually helpful for i915. We end up having to root around
in the display_info in a lot of places anyway. So a helper for single
boolean seems a bit out of place perhaps.
--
Ville Syrjälä
Intel
Re: [Freedreno] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, 15 Oct 2021, Claudio Suarez wrote:
> 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_connector.c | 5 +
> drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> b/drivers/gpu/drm/i915/display/intel_connector.c
> index 9bed1ccecea0..3346b55df6e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
> return ret;
> }
>
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> +{
> + return connector->display_info.is_hdmi;
> +}
> +
A helper like this belongs in drm, not i915. Seems useful in other
drivers too.
BR,
Jani.
> static const struct drm_prop_enum_list force_audio_names[] = {
> { HDMI_AUDIO_OFF_DVI, "force-dvi" },
> { HDMI_AUDIO_OFF, "off" },
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
> b/drivers/gpu/drm/i915/display/intel_connector.h
> index 661a37a3c6d8..ceda6e72ece6 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.h
> +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector
> *connector);
> int intel_connector_update_modes(struct drm_connector *connector,
>struct edid *edid);
> int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter
> *adapter);
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
> void intel_attach_force_audio_property(struct drm_connector *connector);
> void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b04685bb6439..2b1d7c5bebdd 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 =
> intel_connector_is_hdmi_monitor(connector);
>
> connected = true;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 6cb27599ea03..a32279e4fee8 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 =
> +
> intel_connector_is_hdmi_monitor(connector);
> }
> } else
> status = connector_status_disconnected;
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Freedreno] [Intel-gfx] [PATCH 15/15] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 15, 2021 at 01:37:13PM +0200, Claudio Suarez wrote:
> 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_connector.c | 5 +
> drivers/gpu/drm/i915/display/intel_connector.h | 1 +
> drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> drivers/gpu/drm/i915/display/intel_sdvo.c | 3 ++-
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c
> b/drivers/gpu/drm/i915/display/intel_connector.c
> index 9bed1ccecea0..3346b55df6e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -213,6 +213,11 @@ int intel_ddc_get_modes(struct drm_connector *connector,
> return ret;
> }
>
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector)
> +{
> + return connector->display_info.is_hdmi;
> +}
> +
> static const struct drm_prop_enum_list force_audio_names[] = {
> { HDMI_AUDIO_OFF_DVI, "force-dvi" },
> { HDMI_AUDIO_OFF, "off" },
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.h
> b/drivers/gpu/drm/i915/display/intel_connector.h
> index 661a37a3c6d8..ceda6e72ece6 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.h
> +++ b/drivers/gpu/drm/i915/display/intel_connector.h
> @@ -27,6 +27,7 @@ enum pipe intel_connector_get_pipe(struct intel_connector
> *connector);
> int intel_connector_update_modes(struct drm_connector *connector,
>struct edid *edid);
> int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter
> *adapter);
> +bool intel_connector_is_hdmi_monitor(struct drm_connector *connector);
> void intel_attach_force_audio_property(struct drm_connector *connector);
> void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> void intel_attach_aspect_ratio_property(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index b04685bb6439..2b1d7c5bebdd 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 =
> intel_connector_is_hdmi_monitor(connector);
Hmm. Have we parse the EDID by this point actually? I don't think that
was the case in the past but maybe it changed at some point.
>
> connected = true;
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 6cb27599ea03..a32279e4fee8 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 =
> +
> intel_connector_is_hdmi_monitor(connector);
FYI there's a third copy of this in intel_dp.c
> }
> } else
> status = connector_status_disconnected;
> --
> 2.33.0
>
--
Ville Syrjälä
Intel
Re: [Freedreno] [PATCH 01/15] gpu/drm: make drm_add_edid_modes() consistent when updating connector->display_info
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 &drm_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.
> drm_warn(connector->dev, "%s: EDID invalid.\n",
>connector->name);
> return 0;
> --
> 2.33.0
>
>
--
Ville Syrjälä
Intel
