On Wed, Jan 20, 2016 at 06:26:16PM -0800, tom.orourke@xxxxxxxxx wrote:
> From: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
>
> GuC SLPC need to be sent data related to Active pipes, refresh rates,
> widi pipes, fullscreen pipes related via host to GuC display mode
> change event.
> This patch defines the event and implements trigger of the event.
>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Acked-by: Tom O'Rourke <Tom.O'Rourke@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 +
> drivers/gpu/drm/i915/intel_slpc.c | 92 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_slpc.h | 1 +
> 3 files changed, 95 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 06ab6df..7c3d902 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13638,6 +13638,8 @@ static int intel_atomic_commit(struct drm_device *dev,
> */
> intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
>
> + intel_slpc_update_display_mode_info(dev);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
> index f155d88..f5f7cad 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.c
> +++ b/drivers/gpu/drm/i915/intel_slpc.c
> @@ -74,6 +74,27 @@ static int host2guc_slpc_shutdown(struct drm_i915_private *dev_priv)
> return ret;
> }
>
> +static int host2guc_slpc_display_mode_change(struct drm_i915_private *dev_priv)
> +{
> + u32 data[7];
> + int ret, i;
> +
> + data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
> + data[1] = SLPC_EVENT(SLPC_EVENT_DISPLAY_MODE_CHANGE, MAX_NUM_OF_PIPE + 1);
> + data[2] = dev_priv->guc.slpc.display_mode_params.global_data;
> + for(i = 0; i < MAX_NUM_OF_PIPE; ++i)
> + data[3+i] = dev_priv->guc.slpc.display_mode_params.per_pipe_info[i].data;
> +
> + ret = host2guc_action(&dev_priv->guc, data, 7);
> +
> + if (0 == ret) {
> + ret = I915_READ(SOFT_SCRATCH(1));
> + ret &= 0xFF;
> + }
> +
> + return ret;
> +}
> +
> static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
> {
> struct drm_device *dev = obj->base.dev;
> @@ -225,3 +246,74 @@ int intel_slpc_reset(struct drm_device *dev)
>
> return ret;
> }
> +
> +int intel_slpc_update_display_mode_info(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_crtc *crtc;
> + struct intel_crtc *intel_crtc;
> + struct drm_connector *connector;
> + struct intel_connector *intel_connector;
> + struct drm_encoder *encoder;
> + struct intel_encoder *intel_encoder;
> + struct intel_display_pipe_basic_info *per_pipe_info;
> + struct intel_slpc_display_mode_event_params *cur_params, old_params;
> + bool notify = false;
> +
> + if (!HAS_SLPC(dev))
> + return -EINVAL;
> +
> + cur_params = &dev_priv->guc.slpc.display_mode_params;
> +
> + /* Copy display mode parameters for comparison */
> + old_params.global_data = cur_params->global_data;
> + cur_params->global_data = 0;
> +
> + for_each_intel_crtc(dev, intel_crtc) {
> + per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
> + old_params.per_pipe_info[intel_crtc->pipe].data = per_pipe_info->data;
> + per_pipe_info->data = 0;
> + }
> +
> + for_each_intel_crtc(dev, intel_crtc) {
> + struct intel_crtc_state *pipe_config;
> +
> + per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
> + crtc = &intel_crtc->base;
> + pipe_config = to_intel_crtc_state(crtc->state);
> +
> + if (pipe_config->base.active) {
> + for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> + encoder = &intel_encoder->base;
> + for_each_connector_on_encoder(dev, encoder, intel_connector) {
> + connector = &intel_connector->base;
> + if (connector->status == connector_status_connected) {
The connecotr/encoder stuff should not be here. We can enable a pipe
without any connected connectors. Also locking is fail.
The patch also fails to explain why the guc needs to know any of this
stuff. It makes me very suspicious that the guc is going to start
poking at some display stuff behind the driver's back. I actually
think that doing some display stuff in the guc might be neat, but not
when it's a black box blob.
> + struct drm_display_mode *mode = &crtc->mode;
> + /* FIXME: Update is_widi based on encoder */
> + per_pipe_info->is_widi = 0;
> + per_pipe_info->refresh_rate = mode->vrefresh;
> + per_pipe_info->vsync_ft_usec = 1000000 / mode->vrefresh;
> + cur_params->active_pipes_bitmask |= (1 << intel_crtc->pipe);
> + cur_params->vbi_sync_on_pipes |= (1 << intel_crtc->pipe);
> + cur_params->num_active_pipes++;
> + }
> + }
> + }
> + }
> + }
> +
> + /* Compare old display mode with current mode. Notify SLPC if it is changed. */
> + if (cur_params->global_data != old_params.global_data)
> + notify = true;
> +
> + for_each_intel_crtc(dev, intel_crtc) {
> + per_pipe_info = &cur_params->per_pipe_info[intel_crtc->pipe];
> + if (old_params.per_pipe_info[intel_crtc->pipe].data != per_pipe_info->data)
> + notify = true;
> + }
> +
> + if (notify)
> + host2guc_slpc_display_mode_change(dev_priv);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h
> index 9673a63..18e551b 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.h
> +++ b/drivers/gpu/drm/i915/intel_slpc.h
> @@ -152,5 +152,6 @@ int intel_slpc_suspend(struct drm_device *dev);
> int intel_slpc_disable(struct drm_device *dev);
> int intel_slpc_enable(struct drm_device *dev);
> int intel_slpc_reset(struct drm_device *dev);
> +int intel_slpc_update_display_mode_info(struct drm_device *dev);
>
> #endif
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
