Re: [PATCH] KMS: cache the EDID information of the LVDS
On Wed, 01 Apr 2009 12:29:04 -0700 Eric Anholt e...@anholt.net wrote: On Fri, 2009-03-27 at 22:01 -0700, Arjan van de Ven wrote: On Fri, 27 Mar 2009 16:08:18 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: return ret; I definitely like the idea of caching the LVDS EDID, but rather than adding it to the intel_output struct I'd rather see it stored with the rest of the LVDS stuff in i915_private (though that stuff should be pulled out into an output private at some point). We could also just open code the intel_ddc_get_modes stuff in intel_lvds_init, getting the EDID just once, storing the property once, and adding the mode list once. intel_lvds_get_modes would need to be fixed too... that gets a bit messy though, esp if other output types decide to also want to cache. Remember that some of this code gets called from DRM or even userland, so the edid will end up being stored in a generic structure Ack. SDVO LVDS and eDP would also want to do this, and i915_private then seems like the wrong place. Yeah after looking again (and fixing this issue myself having forgotten about this), I ack it. Wanna pull it in Eric? Acked-by: Jesse Barnes jbar...@virtuousgeek.org -- Jesse Barnes, Intel Open Source Technology Center -- Register Now Save for Velocity, the Web Performance Operations Conference from O'Reilly Media. Velocity features a full day of expert-led, hands-on workshops and two days of sessions from industry leaders in dedicated Performance Operations tracks. Use code vel09scf and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] KMS: cache the EDID information of the LVDS
On Fri, 2009-03-27 at 22:01 -0700, Arjan van de Ven wrote: On Fri, 27 Mar 2009 16:08:18 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: return ret; I definitely like the idea of caching the LVDS EDID, but rather than adding it to the intel_output struct I'd rather see it stored with the rest of the LVDS stuff in i915_private (though that stuff should be pulled out into an output private at some point). We could also just open code the intel_ddc_get_modes stuff in intel_lvds_init, getting the EDID just once, storing the property once, and adding the mode list once. intel_lvds_get_modes would need to be fixed too... that gets a bit messy though, esp if other output types decide to also want to cache. Remember that some of this code gets called from DRM or even userland, so the edid will end up being stored in a generic structure Ack. SDVO LVDS and eDP would also want to do this, and i915_private then seems like the wrong place. -- Eric Anholt e...@anholt.net eric.anh...@intel.com signature.asc Description: This is a digitally signed message part -- -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] KMS: cache the EDID information of the LVDS
On Mon, 23 Mar 2009 13:48:33 -0700
Arjan van de Ven ar...@infradead.org wrote:
From eb512d6d953c8acc8abe1048862a92cec58d9791 Mon Sep 17 00:00:00
2001
From: Arjan van de Ven ar...@linux.intel.com
Date: Mon, 23 Mar 2009 13:38:49 -0700
Subject: [PATCH] KMS: cache the EDID information of the LVDS
you're not going to replace your LVDS at runtime. so this patch
caches the EDID information of the LVDS. An LVDS probe can easily take
200 milliseconds, and we do this multiple times during a system
startup.
Signed-off-by: Arjan van de Ven ar...@linux.intel.com
---
drivers/gpu/drm/i915/intel_drv.h |1 +
drivers/gpu/drm/i915/intel_lvds.c |2 ++
drivers/gpu/drm/i915/intel_modes.c |8 +++-
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h index 957daef..22a74bd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -81,6 +81,7 @@ struct intel_output {
int type;
struct intel_i2c_chan *i2c_bus; /* for control functions */
struct intel_i2c_chan *ddc_bus; /* for DDC only stuff */
+ struct edid *edid;
bool load_detect_temp;
bool needs_tv_clock;
void *dev_priv;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c index 0d211af..dc4fecc 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -336,6 +336,7 @@ static void intel_lvds_destroy(struct
drm_connector *connector) intel_i2c_destroy(intel_output-ddc_bus);
drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
+ kfree(intel_output-edid);
kfree(connector);
}
@@ -516,5 +517,6 @@ failed:
if (intel_output-ddc_bus)
intel_i2c_destroy(intel_output-ddc_bus);
drm_connector_cleanup(connector);
+ kfree(intel_output-edid);
kfree(connector);
}
diff --git a/drivers/gpu/drm/i915/intel_modes.c
b/drivers/gpu/drm/i915/intel_modes.c index e42019e..83816a4 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -70,13 +70,19 @@ int intel_ddc_get_modes(struct intel_output
*intel_output) struct edid *edid;
int ret = 0;
+ if (intel_output-edid)
+ return ret;
+
edid = drm_get_edid(intel_output-base,
intel_output-ddc_bus-adapter);
if (edid) {
drm_mode_connector_update_edid_property(intel_output-base,
edid);
ret = drm_add_edid_modes(intel_output-base, edid);
- kfree(edid);
+ if (intel_output-type == INTEL_OUTPUT_LVDS)
+ intel_output-edid = edid;
+ else
+ kfree(edid);
}
return ret;
I definitely like the idea of caching the LVDS EDID, but rather than
adding it to the intel_output struct I'd rather see it stored with the
rest of the LVDS stuff in i915_private (though that stuff should be
pulled out into an output private at some point). We could also just
open code the intel_ddc_get_modes stuff in intel_lvds_init, getting the
EDID just once, storing the property once, and adding the mode list
once. intel_lvds_get_modes would need to be fixed too...
--
Jesse Barnes, Intel Open Source Technology Center
--
--
___
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] KMS: cache the EDID information of the LVDS
On Fri, 27 Mar 2009 16:08:18 -0700 Jesse Barnes jbar...@virtuousgeek.org wrote: return ret; I definitely like the idea of caching the LVDS EDID, but rather than adding it to the intel_output struct I'd rather see it stored with the rest of the LVDS stuff in i915_private (though that stuff should be pulled out into an output private at some point). We could also just open code the intel_ddc_get_modes stuff in intel_lvds_init, getting the EDID just once, storing the property once, and adding the mode list once. intel_lvds_get_modes would need to be fixed too... that gets a bit messy though, esp if other output types decide to also want to cache. Remember that some of this code gets called from DRM or even userland, so the edid will end up being stored in a generic structure -- Arjan van de VenIntel Open Source Technology Centre For development, discussion and tips for power savings, visit http://www.lesswatts.org -- -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
