Re: [PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>]

On 3/27/2015 2:02 PM, Daniel Vetter wrote:
> On Fri, Mar 27, 2015 at 11:37:09AM +0530, Sivakumar Thulasimani wrote:
>>
>> On 3/27/2015 12:50 AM, Rodrigo Vivi wrote:
>>> Let's know beforehand if PSR is ready and will be enabled so we can
>>> prevent DRRS to get enabled.
>>>
>>> v2: Removing is_edp_psr func that is not used after this patch.
>>>      Rename match_conditions and document it since it is now external.
>>>      Moving to a propper place as pointed out by Sivakumar.
>>>      Use a better name as pointed out by Ram.
>>>
>>> Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
>>> Cc: Ramalingam C <ramalingam.c@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_display.c |  1 +
>>>   drivers/gpu/drm/i915/intel_dp.c      |  2 ++
>>>   drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>>   drivers/gpu/drm/i915/intel_psr.c     | 50 +++++++++++++++++++++---------------
>>>   4 files changed, 35 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 35cdb48..9112ad9 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -10367,6 +10367,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>>>   		      pipe_config->pch_pfit.pos,
>>>   		      pipe_config->pch_pfit.size,
>>>   		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
>>> +	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
>>>   	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
>>>   	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
>>>   }
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 637dd53..e6b1c42 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -1388,6 +1388,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>>   		 */
>>>   		min_lane_count = max_lane_count;
>>>   		min_clock = max_clock;
>>> +
>>> +		pipe_config->psr_ready = intel_psr_ready(intel_dp);
>>>   	}
>>>   	for (; bpp >= 6*3; bpp -= 2*3) {
>> This is still updated during every modeset. since PSR is specific to eDP,
>> can this new variable be stored inside intel_dp and be updated in
>> intel_edp_init_connector itself ? that way we will initialize this during
>> driver load and can reuse it forever.
> Imo that's totally fine. I actually prefer to recompute state instead of
> storing it, since that cuts out one indirection. I'm totally fine with
> this approach here.
> -Daniel
just checked out intel_psr_match_conditions (which is renamed here) and 
it is checking for S3D modes, so yes in that case it is better to have 
this check here. but the same function also has check for interlaced 
mode. i am yet to come across an edp panel with Interlaced mode, that 
can be removed may be sometime later in a different patch.

regards,
Sivakumar Thulasimani
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx