On Tue, Nov 07, 2017 at 06:38:23PM +0000, Ramalingam C wrote:
> From: "C, Ramalingam" <ramalingam.c@xxxxxxxxx>
>
> Debugfs called i915_drrs_ctl is added to enable and disable the
> eDP DRRS. Writing 0 will disable the feature, whereas non-zero
> will enable the feature.
>
> Possibility of disabling the DRRS, enables the testing of the
> frontbuffer tracking based features (FBC, DRRS and PSR) as
> standalone or any combination of the set.
>
> [v2]: ctl interface is moved from module parameter to debugfs [Rodrigo]
Thanks
>
> Signed-off-by: C, Ramalingam <ramalingam.c@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 43 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0bb6e01121fc..0c1501fe4c9f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4747,6 +4747,46 @@ static const struct file_operations i915_hpd_storm_ctl_fops = {
> .write = i915_hpd_storm_ctl_write
> };
>
> +static int i915_drrs_ctl_set(void *data, u64 val)
> +{
> + struct drm_i915_private *dev_priv = data;
> + struct drm_device *dev = &dev_priv->drm;
> + struct intel_crtc *intel_crtc;
> + struct intel_encoder *encoder;
> + struct intel_dp *intel_dp;
> +
> + if (INTEL_GEN(dev_priv) < 7)
I believe we need to define a HAS_DRRS(dev_priv)
which based on what is on intel_cpu_transcoder_set_m_n would be
(IS_CHERRYVIEW(dev_priv) || INTEL_GEN(dev_priv) < 8)
> + return -ENODEV;
> +
> + drm_modeset_lock_all(dev);
my first reaction to this was: "why do you need to lock all modeset?!"
But this simplify a lot the logic here...
This assure that drrs is really not getting changed from other places.
> + for_each_intel_crtc(dev, intel_crtc) {
> + if (!intel_crtc->base.state->active ||
> + !intel_crtc->config->has_drrs)
I was going to say that this check already happens inside
enable and disable functions... But I see the reason why to
check before the unecessary noise.
> + continue;
> +
> + for_each_encoder_on_crtc(dev, &intel_crtc->base, encoder) {
> + if (encoder->type != INTEL_OUTPUT_EDP)
> + continue;
> +
> + DRM_DEBUG_DRIVER("Manually %sabling DRRS. %llu\n",
> + val ? "en" : "dis", val);
> +
> + intel_dp = enc_to_intel_dp(&encoder->base);
> + if (val)
> + intel_edp_drrs_enable(intel_dp,
> + intel_crtc->config);
> + else
> + intel_edp_drrs_disable(intel_dp,
> + intel_crtc->config);
> + }
> + }
> + drm_modeset_unlock_all(dev);
> +
> + return 0;
> +}
It seems simple and effective. Simpler than I imagined...
My only question is about that HAS_DRRS and when to skip here...
> +
> +DEFINE_SIMPLE_ATTRIBUTE(i915_drrs_ctl_fops, NULL, i915_drrs_ctl_set, "%llu\n");
> +
> static const struct drm_info_list i915_debugfs_list[] = {
> {"i915_capabilities", i915_capabilities, 0},
> {"i915_gem_objects", i915_gem_object_info, 0},
> @@ -4828,7 +4868,8 @@ static const struct i915_debugfs_files {
> {"i915_dp_test_active", &i915_displayport_test_active_fops},
> {"i915_guc_log_control", &i915_guc_log_control_fops},
> {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops},
> - {"i915_ipc_status", &i915_ipc_status_fops}
> + {"i915_ipc_status", &i915_ipc_status_fops},
> + {"i915_drrs_ctl", &i915_drrs_ctl_fops}
> };
>
> int i915_debugfs_register(struct drm_i915_private *dev_priv)
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
