Re: [RFC PATCH 11/18] drm/i915: Updating the crtc modes in DRRS transitions

[Ramalingam C <ramalingam.c@intel.com>]

On Friday 26 June 2015 10:41 PM, Daniel Vetter wrote:
> On Fri, Jun 26, 2015 at 07:21:55PM +0530, Ramalingam C wrote:
>> During the DRRS state transitions we are modifying the clock and
>> hence the vrefresh rate.
>>
>> So we need to update the drm_crtc->mode and the adjusted
>> mode in intel_crtc. So that watermark calculations will be as per the
>> new modified clock.
>>
>> Signed-off-by: Ramalingam C<ramalingam.c@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_drrs.c     |   14 ++++++++++++++
>>   drivers/gpu/drm/i915/intel_dsi_drrs.c |   14 +++++++++++++-
>>   2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drrs.c b/drivers/gpu/drm/i915/intel_drrs.c
>> index 42b420d..2901832 100644
>> --- a/drivers/gpu/drm/i915/intel_drrs.c
>> +++ b/drivers/gpu/drm/i915/intel_drrs.c
>> @@ -169,6 +169,20 @@ void intel_set_drrs_state(struct i915_drrs *drrs)
>>   		 * If it is non-DSI encoders.
>>   		 * As only DSI has SEAMLESS_DRRS_SUPPORT_SW.
>>   		 */
>> +		/*
>> +		 * TODO: Protect the access to the crtc mode with corresponding
>> +		 * mutex in case of Idleness DRRS. As media playback update
>> +		 * will happen under crtc modeset lock protection
>> +		 */
>> +		drm_modeset_lock(&intel_crtc->base.mutex, NULL);
>> +		intel_crtc->base.mode.clock = target_mode->clock;
>> +		intel_crtc->base.mode.vrefresh = target_mode->vrefresh;
>> +		intel_crtc->config->base.adjusted_mode.clock =
>> +							target_mode->clock;
>> +		intel_crtc->config->base.adjusted_mode.vrefresh =
>> +							target_mode->vrefresh;
>> +		drm_modeset_unlock(&intel_crtc->base.mutex);
> No. For video DRRS (i.e. changing the vrefresh rate to exactly what we
> want to match media content) userspace needs to do a full modeset. Maarten
> is working on infrastructure to avoid full modeset in some special mode
> changes (to implement the fast pfit change from Jesse's original fastboot
> work). And we can extend this to DRRS too.
Daniel,

But this interface that we have developed for android is not just for 
video playback.
We are trying to provide a interface where user space can request for 
custom vrefresh,
if the rate of the display content doesn't need the vrefresh of current 
applied mode.
For example scenarios like text editor or some game where lower Frame 
per second is sufficient. And if the panel supports
the seamless drrs we don't need to do a modeset in such scenarios.

Either way Please do share/point to the details of the Maarten design. 
So that we can see if we are
working on same area and if so how far we can reuse each others work.
> For seamless&transparent DRRS we can still keep the current design with
> effectively 2 dotclocks.
You mean 2 dotclocks at drm_display_mode or drrs structure?
>
> Also with atomic you can combine the clock change with the first video
> frame which is double-awesome ;-)
> -Daniel
>
>> +
>>   		drrs_state->current_rr_type = drrs_state->target_rr_type;
>>   		DRM_INFO("Refresh Rate set to : %dHz\n", refresh_rate);
>>   	}
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_drrs.c b/drivers/gpu/drm/i915/intel_dsi_drrs.c
>> index eb0758a..d4bb70a 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_drrs.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_drrs.c
>> @@ -46,6 +46,7 @@ static void intel_mipi_drrs_work_fn(struct work_struct *__work)
>>   	struct i915_drrs *drrs = work->drrs;
>>   	struct intel_encoder *intel_encoder = drrs->connector->encoder;
>>   	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&intel_encoder->base);
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
>>   	struct dsi_drrs *dsi_drrs = &intel_dsi->dsi_drrs;
>>   	struct dsi_mnp *dsi_mnp;
>>   	struct drm_display_mode *prev_mode = NULL;
>> @@ -69,11 +70,22 @@ retry:
>>   		/* PLL Programming is successful */
>>   		mutex_lock(&drrs->drrs_mutex);
>>   		drrs->drrs_state.current_rr_type = work->target_rr_type;
>> +
>> +		drm_modeset_lock(&intel_crtc->base.mutex, NULL);
>> +		intel_crtc->base.mode.clock = work->target_mode->clock;
>> +		intel_crtc->base.mode.vrefresh = work->target_mode->vrefresh;
>> +		intel_crtc->config->base.adjusted_mode.clock =
>> +						work->target_mode->clock;
>> +		intel_crtc->config->base.adjusted_mode.vrefresh =
>> +						work->target_mode->vrefresh;
>> +		drm_modeset_unlock(&intel_crtc->base.mutex);
>> +
>>   		mutex_unlock(&drrs->drrs_mutex);
>> +
>>   		DRM_INFO("Refresh Rate set to : %dHz\n",
>>   						work->target_mode->vrefresh);
>>   
>> -		/* TODO: Update crtc mode and drain ladency with watermark */
>> +		/* TODO: Update drain ladency with watermark */
>>   
>>   	} else if (ret == -ETIMEDOUT && retry_cnt) {
>>   
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Thanks,
--Ram

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