Re: [PATCH v9 3/6] drm/i915: Add support for DRRS to switch RR

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

On Apr-11-2014 4:51 PM, Vandana Kannan wrote:
> On Apr-11-2014 2:56 PM, Daniel Vetter wrote:
>> On Fri, Apr 11, 2014 at 02:48:53PM +0530, Vandana Kannan wrote:
>>> On Apr-10-2014 2:28 PM, Daniel Vetter wrote:
>>>> On Thu, Apr 10, 2014 at 11:43:15AM +0300, Jani Nikula wrote:
>>>>>
>>>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>>>>
>>>>>
>>>>> On Sat, 05 Apr 2014, Vandana Kannan <vandana.kannan@intel.com> wrote:
>>>>>> From: Pradeep Bhat <pradeep.bhat@intel.com>
>>>>>>
>>>>>> This patch computes and stored 2nd M/N/TU for switching to different
>>>>>> refresh rate dynamically. PIPECONF_EDP_RR_MODE_SWITCH bit helps toggle
>>>>>> between alternate refresh rates programmed in 2nd M/N/TU registers.
>>>>>>
>>>>>> v2: Daniel's review comments
>>>>>> Computing M2/N2 in compute_config and storing it in crtc_config
>>>>>>
>>>>>> v3: Modified reference to edp_downclock and edp_downclock_avail based on the
>>>>>> changes made to move them from dev_private to intel_panel.
>>>>>>
>>>>>> v4: Modified references to is_drrs_supported based on the changes made to
>>>>>> rename it to drrs_support.
>>>>>>
>>>>>> v5: Jani's review comments
>>>>>> Removed superfluous return statements. Changed support for Gen 7 and above.
>>>>>> Corrected indentation. Re-structured the code which finds crtc and connector
>>>>>> from encoder. Changed some logs to be less verbose.
>>>>>>
>>>>>> v6: Modifying i915_drrs to include only intel connector as intel_dp can be
>>>>>> derived from intel connector when required.
>>>>>>
>>>>>> v7: As per internal review comments, acquiring mutex just before accessing
>>>>>> drrs RR. As per Chris's review comments, added documentation about the use
>>>>>> of locking in the function.
>>>>>>
>>>>>> v8: Incorporated Jani's review comments.
>>>>>> Removed reference to edp_downclock.
>>>>>>
>>>>>> v9: Jani's review comments. Modified comment in set_drrs. Changed index to
>>>>>> type edp_drrs_refresh_rate_type. Check if PSR is enabled before setting
>>>>>> registers fo DRRS.
>>>>>>
>>>>>> Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
>>>>>> Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
>>>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>>>
>>>> Queued for -next, thanks for the patch. One thing that's missing though is
>>>> the state readout and cross-check support for this new bit of crtc->config
>>>> data. We need to add that before putting this to real use.
>>>> -Daniel
>>>>
>>> Hi Daniel,
>>>
>>> Could you please elaborate on your input above - on the missing code and
>>> cross-checking for support part?
>>
>> If you add new state to crtc->config then you need to add the relevan
>> readout code for that state (see the code in check_crtc_state) and ofc
>> also add it to intel_pipe_config_compare.
>>
>> This is a debug feature of our driver to make sure we never lose track of
>> things and thus far has been extremely helpful in catching issues early.
>> -Daniel
>>
> I have submitted a patch adding this piece of code. Please have a look..
> http://lists.freedesktop.org/archives/intel-gfx/2014-April/043569.html
> 
> - Vandana
> 
Hi Daniel,
Please help review the patch
http://lists.freedesktop.org/archives/intel-gfx/2014-April/043569.html
which is based on your review comments above.
-Vandana
_______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>