| Message ID | 1435326722-24633-12-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. For seamless&transparent DRRS we can still keep the current design with effectively 2 dotclocks. 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
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
On Mon, Jun 29, 2015 at 08:28:53PM +0530, Ramalingam C wrote: > > 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. Well I just called it video DRRS since I thought that's the main use-case. The interface I'm proposing here (which already exists, that's the point) will of course work no matter what exactly you're displaying. > 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. Maarten is still debugging a pile of fallout in unrelated code, probably best if you ping him directly on irc about the latest stuff. > >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? In intel_crtc_config we have two sets of DP m/n values, which means 2 different clocks for the sink. We also have pipe_config->port_clock. Just need to extend that to low_port_clock for seamless DRRS (i.e. managed by the kernel). -Daniel
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); + 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) {
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(-)