Patchwork [Maverick] drm/radeon: Update LVDS connector edid_property only once

login
register
mail settings
Submitter Alberto Milone
Date June 24, 2010, 2:58 p.m.
Message ID <AANLkTikt8Il9tjYhYWBDW7WASLARYOhb1MeKkH4niV_N@mail.gmail.com>
Download mbox | patch
Permalink /patch/56797/
State Rejected
Delegated to: Leann Ogasawara
Headers show

Comments

Alberto Milone - June 24, 2010, 2:58 p.m.
Hi all,

After looking at Arjan's patch to cache the EDID for the intel driver,
I decided to write a similar patch which should help us reduce boot
time with radeon.

Please consider adopting my patch in Maverick's kernel.

Note: my patch is not the upstream code, read here for further details:
http://lists.freedesktop.org/archives/dri-devel/2010-May/000948.html

Thanks in advance for your time.
Chase Douglas - June 24, 2010, 3:49 p.m.
On Thu, 2010-06-24 at 16:58 +0200, Alberto Milone wrote:
> Hi all,
> 
> After looking at Arjan's patch to cache the EDID for the intel driver,
> I decided to write a similar patch which should help us reduce boot
> time with radeon.
> 
> Please consider adopting my patch in Maverick's kernel.
> 
> Note: my patch is not the upstream code, read here for further details:
> http://lists.freedesktop.org/archives/dri-devel/2010-May/000948.html

Do you know why it's not upstream? I didn't see any replies to your
post. If it's just a matter of not receiving any replies, it sometimes
takes a resend or two for people to notice.

The patch seems to make sense, but I would be worried about adding a
commit that diverges us from upstream. As we've seen in places like the
dell x86 platform driver, as soon as we start making ubuntu-only changes
that don't get upstream, ongoing maintenance becomes a larger burden.
There's still time to get upstream to pick up the patch before we need
to lock down the kernel for feature freeze.

I'm curious how much time this saves. Do you have any statistics?

Thanks,

-- Chase
Alberto Milone - June 24, 2010, 3:50 p.m.
On 24 June 2010 16:58, Alberto Milone <alberto.milone@canonical.com> wrote:
> Hi all,
>
> After looking at Arjan's patch to cache the EDID for the intel driver,
> I decided to write a similar patch which should help us reduce boot
> time with radeon.
>
> Please consider adopting my patch in Maverick's kernel.
>
> Note: my patch is not the upstream code, read here for further details:
> http://lists.freedesktop.org/archives/dri-devel/2010-May/000948.html
>
> Thanks in advance for your time.
>

There seem to be some screen blanking issues on boot (visible with a
bootsplash) that I can't reproduce here. I'll investigate the issue.

Regards,

Patch

From 0bc3de352d2ed4356a51ca712e279d0058374b45 Mon Sep 17 00:00:00 2001
From: Alberto Milone <alberto.milone@canonical.com>
Date: Mon, 31 May 2010 14:20:06 +0200
Subject: [PATCH]     drm/edid: Update LVDS connector edid_property only once.

    drm_mode_connector_update_edid_property is called every time
    radeon_ddc_get_modes is called. Due to the nature of LVDS, it
    should be sufficient to update edid_property only once.

Signed-off-by: Alberto Milone <alberto.milone@canonical.com>
---
 drivers/gpu/drm/radeon/radeon_connectors.c |    1 +
 drivers/gpu/drm/radeon/radeon_display.c    |    8 ++++++++
 drivers/gpu/drm/radeon/radeon_mode.h       |    1 +
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 0c7ccc6..389a9dd 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1068,6 +1068,7 @@  radeon_add_atom_connector(struct drm_device *dev,
 	connector = &radeon_connector->base;
 
 	radeon_connector->connector_id = connector_id;
+	radeon_connector->edid_property_updated = 0;
 	radeon_connector->devices = supported_device;
 	radeon_connector->shared_ddc = shared_ddc;
 	radeon_connector->connector_object_id = connector_object_id;
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 1006549..058602b 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -403,6 +403,12 @@  int radeon_ddc_get_modes(struct radeon_connector *radeon_connector)
 		     dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) && dig->dp_i2c_bus)
 			radeon_connector->edid = drm_get_edid(&radeon_connector->base, &dig->dp_i2c_bus->adapter);
 	}
+	if (radeon_connector->edid_property_updated &&
+	    radeon_connector->base.connector_type == DRM_MODE_CONNECTOR_LVDS) {
+		DRM_INFO("Skipping update of EDID property due to cached property\n");
+		return ret;
+	}
+
 	if (!radeon_connector->ddc_bus)
 		return -1;
 	if (!radeon_connector->edid) {
@@ -411,6 +417,8 @@  int radeon_ddc_get_modes(struct radeon_connector *radeon_connector)
 	/* some servers provide a hardcoded edid in rom for KVMs */
 	if (!radeon_connector->edid)
 		radeon_connector->edid = radeon_combios_get_hardcoded_edid(rdev);
+
+	radeon_connector->edid_property_updated = 1;
 	if (radeon_connector->edid) {
 		drm_mode_connector_update_edid_property(&radeon_connector->base, radeon_connector->edid);
 		ret = drm_add_edid_modes(&radeon_connector->base, radeon_connector->edid);
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 67358ba..e5f00e8 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -397,6 +397,7 @@  struct radeon_connector {
 	/* we need to mind the EDID between detect
 	   and get modes due to analog/digital/tvencoder */
 	struct edid *edid;
+	unsigned int edid_property_updated;
 	void *con_priv;
 	bool dac_load_detect;
 	uint16_t connector_object_id;
-- 
1.7.0.4