From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vandana Kannan Subject: Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature Date: Wed, 18 Dec 2013 15:43:34 +0530 Message-ID: <52B1754E.8070000@intel.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> <20131218091104.GA22448@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 78E5FFBFCB for ; Wed, 18 Dec 2013 02:14:04 -0800 (PST) In-Reply-To: <20131218091104.GA22448@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Chris Wilson Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Dec-18-2013 2:41 PM, Chris Wilson wrote: > 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/. > I will make these changes. - Vandana >>>> @@ -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 >