From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature Date: Wed, 18 Dec 2013 09:11:04 +0000 Message-ID: <20131218091104.GA22448@nuc-i3427.alporthouse.com> References: <1387258107-19232-1-git-send-email-vandana.kannan@intel.com> <1387258107-19232-2-git-send-email-vandana.kannan@intel.com> <20131217122637.GG22448@nuc-i3427.alporthouse.com> <52B1580C.3010300@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id 9FCA3FA809 for ; Wed, 18 Dec 2013 01:11:07 -0800 (PST) Content-Disposition: inline In-Reply-To: <52B1580C.3010300@intel.com> 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: Vandana Kannan Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Wed, Dec 18, 2013 at 01:38:44PM +0530, Vandana Kannan wrote: > On Dec-17-2013 5:56 PM, Chris Wilson wrote: > > On Tue, Dec 17, 2013 at 10:58:23AM +0530, Vandana Kannan wrote: > >> From: Pradeep Bhat > >> > >> This patch reads the DRRS support and Mode type from VBT fields. > >> The read information will be stored in VBT struct during BIOS > >> parsing. The above functionality is needed for decision making > >> whether DRRS feature is supported in i915 driver for eDP panels. > >> This information helps us decide if seamless DRRS can be done > >> at runtime to support certain power saving features. This patch > >> was tested by setting necessary bit in VBT struct and merging > >> the new VBT with system BIOS so that we can read the value. > > > > What's the reason for the inconsistent intel_ tautology? > > > If you are referring to the names of members under bdb_driver_features, > which start with intel_, then this is something which can be modified to > remove the "intel_". Is it ok? Yes, we use intel_ as a function prefix for generic routines that apply to almost all display engines. We expect that all of our hardware specific structure are used for Intel and so don't need reminding, least of all, inconsistently. And s/drrs_state/drrs/. > >> @@ -488,6 +508,9 @@ parse_driver_features(struct drm_i915_private *dev_priv, > >> > >> if (driver->dual_frequency) > >> dev_priv->render_reclock_avail = true; > >> + > >> + dev_priv->vbt.intel_drrs_enabled = driver->intel_drrs_state; > >> + DRM_DEBUG_KMS("DRRS State Enabled : %d\n", driver->intel_drrs_state); > > > > Now this oddity needs a big explanation. Writing that will hopefully > > reveal a better strategy. > > -Chris > > > Panel vendors specify panel capabilities via the VBT. Following this, > the panel's capability to support seamless DRRS has to be read from the > VBT. The same is being done in the piece of code above. Without this we > cannot assume that the panel supports seamless DRRS. Nevermind, I misread driver-> as dev_priv->. -Chris -- Chris Wilson, Intel Open Source Technology Centre