The same definition of DDC_SEGMENT_ADDR is currently defined in two places, drm_edid.c and inno_hdmi.h. Let's consolidate the definition into drm_edid.h in the same way that DDC_ADDR is defined.
Signed-off-by: Shawn Guo shawnguo@kernel.org --- drivers/gpu/drm/drm_edid.c | 1 - drivers/gpu/drm/rockchip/inno_hdmi.h | 2 -- include/drm/drm_edid.h | 1 + 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7df26d4b7ad8..9d6735b68152 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1191,7 +1191,6 @@ bool drm_edid_is_valid(struct edid *edid) } EXPORT_SYMBOL(drm_edid_is_valid);
-#define DDC_SEGMENT_ADDR 0x30 /** * drm_do_probe_ddc_edid() - get EDID information via I2C * @data: I2C device adapter diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.h b/drivers/gpu/drm/rockchip/inno_hdmi.h index aa7c415f8cc1..b27b30934c7e 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.h +++ b/drivers/gpu/drm/rockchip/inno_hdmi.h @@ -16,8 +16,6 @@ #ifndef __INNO_HDMI_H__ #define __INNO_HDMI_H__
-#define DDC_SEGMENT_ADDR 0x30 - enum PWR_MODE { NORMAL, LOWER_PWR, diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 919933d1beb4..7ee78d5c9252 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -26,6 +26,7 @@ #include <linux/types.h>
#define EDID_LENGTH 128 +#define DDC_SEGMENT_ADDR 0x30 #define DDC_ADDR 0x50 #define DDC_ADDR2 0x52 /* E-DDC 1.2 - where DisplayID can hide */
On Thu, Jul 14, 2016 at 05:00:28PM +0800, Shawn Guo wrote:
The same definition of DDC_SEGMENT_ADDR is currently defined in two places, drm_edid.c and inno_hdmi.h. Let's consolidate the definition into drm_edid.h in the same way that DDC_ADDR is defined.
Signed-off-by: Shawn Guo shawnguo@kernel.org
What really should be done here is nuke the fake i2c adapter in inno_hdmi.c and instead just directly read the edid from the hdim IP. Using drm_get_edid for something that's not backed by a real i2c bus is bonghits. -Daniel
drivers/gpu/drm/drm_edid.c | 1 - drivers/gpu/drm/rockchip/inno_hdmi.h | 2 -- include/drm/drm_edid.h | 1 + 3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7df26d4b7ad8..9d6735b68152 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1191,7 +1191,6 @@ bool drm_edid_is_valid(struct edid *edid) } EXPORT_SYMBOL(drm_edid_is_valid);
-#define DDC_SEGMENT_ADDR 0x30 /**
- drm_do_probe_ddc_edid() - get EDID information via I2C
- @data: I2C device adapter
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.h b/drivers/gpu/drm/rockchip/inno_hdmi.h index aa7c415f8cc1..b27b30934c7e 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.h +++ b/drivers/gpu/drm/rockchip/inno_hdmi.h @@ -16,8 +16,6 @@ #ifndef __INNO_HDMI_H__ #define __INNO_HDMI_H__
-#define DDC_SEGMENT_ADDR 0x30
enum PWR_MODE { NORMAL, LOWER_PWR, diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 919933d1beb4..7ee78d5c9252 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -26,6 +26,7 @@ #include <linux/types.h>
#define EDID_LENGTH 128 +#define DDC_SEGMENT_ADDR 0x30 #define DDC_ADDR 0x50 #define DDC_ADDR2 0x52 /* E-DDC 1.2 - where DisplayID can hide */
-- 1.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jul 14, 2016 at 02:45:41PM +0200, Daniel Vetter wrote:
On Thu, Jul 14, 2016 at 05:00:28PM +0800, Shawn Guo wrote:
The same definition of DDC_SEGMENT_ADDR is currently defined in two places, drm_edid.c and inno_hdmi.h. Let's consolidate the definition into drm_edid.h in the same way that DDC_ADDR is defined.
Signed-off-by: Shawn Guo shawnguo@kernel.org
What really should be done here is nuke the fake i2c adapter in inno_hdmi.c and instead just directly read the edid from the hdim IP. Using drm_get_edid for something that's not backed by a real i2c bus is bonghits.
This patch just makes some change literally. I do not understand how the IP block works at all. I thought the HDMI IP talks to monitors using I2C protocol implemented inside the IP block. I added Rockchip folks to see if we can get any clarifications from them.
Shawn
On Thu, Jul 14, 2016 at 09:21:03PM +0800, Shawn Guo wrote:
On Thu, Jul 14, 2016 at 02:45:41PM +0200, Daniel Vetter wrote:
On Thu, Jul 14, 2016 at 05:00:28PM +0800, Shawn Guo wrote:
The same definition of DDC_SEGMENT_ADDR is currently defined in two places, drm_edid.c and inno_hdmi.h. Let's consolidate the definition into drm_edid.h in the same way that DDC_ADDR is defined.
Signed-off-by: Shawn Guo shawnguo@kernel.org
What really should be done here is nuke the fake i2c adapter in inno_hdmi.c and instead just directly read the edid from the hdim IP. Using drm_get_edid for something that's not backed by a real i2c bus is bonghits.
This patch just makes some change literally. I do not understand how the IP block works at all. I thought the HDMI IP talks to monitors using I2C protocol implemented inside the IP block. I added Rockchip folks to see if we can get any clarifications from them.
btw drm_edid.c gained a special variant for this case a while ago: drm_do_get_edid(). With that you can retain all the robustness/retrying logic, while providing your own special function to read an entire edid block. I think that's the function which should be used here. -Daniel
On Fri, Jul 15, 2016 at 09:24:36AM +0200, Daniel Vetter wrote:
On Thu, Jul 14, 2016 at 09:21:03PM +0800, Shawn Guo wrote:
On Thu, Jul 14, 2016 at 02:45:41PM +0200, Daniel Vetter wrote:
On Thu, Jul 14, 2016 at 05:00:28PM +0800, Shawn Guo wrote:
The same definition of DDC_SEGMENT_ADDR is currently defined in two places, drm_edid.c and inno_hdmi.h. Let's consolidate the definition into drm_edid.h in the same way that DDC_ADDR is defined.
Signed-off-by: Shawn Guo shawnguo@kernel.org
What really should be done here is nuke the fake i2c adapter in inno_hdmi.c and instead just directly read the edid from the hdim IP. Using drm_get_edid for something that's not backed by a real i2c bus is bonghits.
This patch just makes some change literally. I do not understand how the IP block works at all. I thought the HDMI IP talks to monitors using I2C protocol implemented inside the IP block. I added Rockchip folks to see if we can get any clarifications from them.
btw drm_edid.c gained a special variant for this case a while ago: drm_do_get_edid(). With that you can retain all the robustness/retrying logic, while providing your own special function to read an entire edid block. I think that's the function which should be used here.
Thanks much for the info, Daniel. It's now clear where I should go for my WIP HDMI driver.
Shawn
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Shawn Guo