[PATCH 1/4] drm/bridge: adv7511: Properly update EDID when no EDID was found

[PATCH 1/4] drm/bridge: adv7511: Properly update EDID when no EDID was found

[Lars-Peter Clausen]

Currently adv7511_get_modes() bails out early when no EDID could be
retrieved. This leaves the previous EDID in place, which is typically not
the intended behavior and might confuse applications. Instead the EDID
should be cleared when no EDID could be retrieved.

All functions that are called after the EDID check handle the case where
the EDID is NULL just fine and exhibit the expected behavior, so just drop
the check.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index b2431aee7887..fb8f4cd29e15 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -591,8 +591,6 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
 
 	kfree(adv7511->edid);
 	adv7511->edid = edid;
-	if (!edid)
-		return 0;
 
 	drm_mode_connector_update_edid_property(connector, edid);
 	count = drm_add_edid_modes(connector, edid);
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/bridge: adv7511: Remove private copy of the EDID

[Lars-Peter Clausen]

The adv7511 driver keeps a private copy of the EDID in its driver state
struct. But this copy is only used in adv7511_get_modes() where it is also
retrieved, so there is no need to keep this extra copy around.

If a need to access the EDID elsewhere in the driver ever arises the copy
that is stored in the connector can be used. This copy is accessible
through drm_connector_get_edid().

Note, this patch removes the NULL check of the EDID before passing it to
drm_detect_hdmi_monitor(), but that is fine since the function correctly
handles the case where the EDID is NULL.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/gpu/drm/bridge/adv7511/adv7511.h     |  2 --
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 16 ++++++----------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index fe18a5d2d84b..12ef2d8ee110 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -328,8 +328,6 @@ struct adv7511 {
 	enum adv7511_sync_polarity hsync_polarity;
 	bool rgb;
 
-	struct edid *edid;
-
 	struct gpio_desc *gpio_pd;
 
 	struct regulator_bulk_data *supplies;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index fb8f4cd29e15..94d598d8aedf 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -199,17 +199,14 @@ static const uint16_t adv7511_csc_ycbcr_to_rgb[] = {
 
 static void adv7511_set_config_csc(struct adv7511 *adv7511,
 				   struct drm_connector *connector,
-				   bool rgb)
+				   bool rgb, bool hdmi_mode)
 {
 	struct adv7511_video_config config;
 	bool output_format_422, output_format_ycbcr;
 	unsigned int mode;
 	uint8_t infoframe[17];
 
-	if (adv7511->edid)
-		config.hdmi_mode = drm_detect_hdmi_monitor(adv7511->edid);
-	else
-		config.hdmi_mode = false;
+	config.hdmi_mode = hdmi_mode;
 
 	hdmi_avi_infoframe_init(&config.avi_infoframe);
 
@@ -589,13 +586,14 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
 	if (!adv7511->powered)
 		__adv7511_power_off(adv7511);
 
-	kfree(adv7511->edid);
-	adv7511->edid = edid;
 
 	drm_mode_connector_update_edid_property(connector, edid);
 	count = drm_add_edid_modes(connector, edid);
 
-	adv7511_set_config_csc(adv7511, connector, adv7511->rgb);
+	adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
+			       drm_detect_hdmi_monitor(edid));
+
+	kfree(edid);
 
 	return count;
 }
@@ -1156,8 +1154,6 @@ static int adv7511_remove(struct i2c_client *i2c)
 
 	i2c_unregister_device(adv7511->i2c_edid);
 
-	kfree(adv7511->edid);
-
 	return 0;
 }
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/bridge: adv7511: Enable connector polling when no interrupt is specified

[Lars-Peter Clausen]

Fall back to polling the connector for connect and disconnect events when
no interrupt is specified. Otherwise these events will not be noticed and
monitor hotplug does not work.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 94d598d8aedf..bd7dbae1119e 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -829,7 +829,11 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge)
 		return -ENODEV;
 	}
 
-	adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	if (adv->i2c_main->irq)
+		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
+				DRM_CONNECTOR_POLL_DISCONNECT;
 
 	ret = drm_connector_init(bridge->dev, &adv->connector,
 				 &adv7511_connector_funcs,
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/bridge: adv7511: Constify HDMI CODEC platform data

[Lars-Peter Clausen]

The HDMI codec platform data is global driver state shared by all
instances. As such it should not be modified (and is not), to make this
explicit declare it as const.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index 67469c26bae8..1b4783d45c53 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -210,7 +210,7 @@ static const struct hdmi_codec_ops adv7511_codec_ops = {
 	.get_dai_id	= adv7511_hdmi_i2s_get_dai_id,
 };
 
-static struct hdmi_codec_pdata codec_data = {
+static const struct hdmi_codec_pdata codec_data = {
 	.ops = &adv7511_codec_ops,
 	.max_i2s_channels = 2,
 	.i2s = 1,
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/bridge: adv7511: Properly update EDID when no EDID was found

[John Stultz]

On Tue, Sep 5, 2017 at 5:10 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> Currently adv7511_get_modes() bails out early when no EDID could be
> retrieved. This leaves the previous EDID in place, which is typically not
> the intended behavior and might confuse applications. Instead the EDID
> should be cleared when no EDID could be retrieved.
>
> All functions that are called after the EDID check handle the case where
> the EDID is NULL just fine and exhibit the expected behavior, so just drop
> the check.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

No objections from me. Ran these on my HiKey board ontop of Linus's
HEAD, and everything seemed ok.

So for the whole patchset:
Tested-by: John Stultz <john.stultz@linaro.org>

thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/bridge: adv7511: Properly update EDID when no EDID was found

[Archit Taneja]

On 09/05/2017 10:14 PM, John Stultz wrote:
> On Tue, Sep 5, 2017 at 5:10 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> Currently adv7511_get_modes() bails out early when no EDID could be
>> retrieved. This leaves the previous EDID in place, which is typically not
>> the intended behavior and might confuse applications. Instead the EDID
>> should be cleared when no EDID could be retrieved.
>>
>> All functions that are called after the EDID check handle the case where
>> the EDID is NULL just fine and exhibit the expected behavior, so just drop
>> the check.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> No objections from me. Ran these on my HiKey board ontop of Linus's
> HEAD, and everything seemed ok.
> 
> So for the whole patchset:
> Tested-by: John Stultz <john.stultz@linaro.org>

The set looks good to me too. I'll queue it to drm-misc-next in a few days.

Thanks,
Archit

> 
> thanks
> -john
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel