From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/i915: cache the EDID for eDP panels Date: Thu, 14 Jun 2012 12:44:53 -0700 Message-ID: <20120614124453.082ea5ef@jbarnes-desktop> References: <1339702113-2961-1-git-send-email-jbarnes@virtuousgeek.org> <1339702946_22343@CP5-2952> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy7-pub.bluehost.com (oproxy7-pub.bluehost.com [67.222.55.9]) by gabe.freedesktop.org (Postfix) with SMTP id 7B711A0B8F for ; Thu, 14 Jun 2012 12:44:55 -0700 (PDT) In-Reply-To: <1339702946_22343@CP5-2952> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 14 Jun 2012 20:42:15 +0100 Chris Wilson wrote: > On Thu, 14 Jun 2012 15:28:33 -0400, Jesse Barnes wrote: > > They aren't going anywhere, and probing on DDC can cause the panel to > > blank briefly, so read them up front and cache them for later queries. > > > > v2: fix potential NULL derefs in intel_dp_get_edid_modes and > > intel_dp_get_edid (Jani) > > copy full EDID length, including extension blocks (Takashi) > > free EDID on teardown (Takashi) > > v3: malloc a new EDID buffer that's big enough for the memcpy (Chris) > > v4: change handling of NULL EDIDs, just preserve the NULL behavior > > across detects and mode list fetches rather than trying to re-fetch > > the EDID (Chris) > > v5: be glad that Chris is around to remind me to hit C-x C-s before > > committing. > > I do 'git add' reminders as well; by appointment only. > > And now to really pour rain on your fire. Why are we repeating quite > obscure logic from intel_modes.c? Can we not export a function to attach > an edid to a connector and then refactor intel_dp.c to only have a > single edid reader where you could add your magic eDP EDID cache > handling in single place. > > So intel_dp_get_edid_modes calls intel_dp_get_edid() and > intel_connector_attach_edid() rather the single-shot > intel_ddc_get_modes(). I totally agree this can be more generic. However, Daniel wanted something for -fixes that's fairly self-contained. I think we should push this for the current kernel (and potentially stable) since it fixes a bug, and your stuff or Daniel's stuff can go into -next that makes every connector better. -- Jesse Barnes, Intel Open Source Technology Center