Re: [PATCH 1/5] drm/i915: Adding VBT fields to support eDP DRRS feature

[Vandana Kannan <vandana.kannan@intel.com>]

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 <pradeep.bhat@intel.com>
>>>>
>>>> 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
>