On Fri, Mar 20, 2015 at 05:04:30PM -0700, Chandra Konduru wrote:
> intel_atomic_setup_scalers sets up scalers based on staged scaling
> requests coming from a crtc and its planes. This function should be
> called from crtc level check path.
> 
> If staged requests are supportable, function assigns scalers to
> requested planes and crtc. This function also takes into account
> the current planes using scalers but not being part of this
> atomic state for optimal operation of scalers. Note that the scaler
> assignement itself is staged into crtc_state and respective
> plane_states for later commit after all checks have been done.
> 
> overall high level flow:
>  - scaler requests are staged into crtc_state by planes/crtc
>  - check whether staged scaling requests can be supported
>  - add planes using scalers that aren't in current transaction
>  - assign scalers to requested users
>  - as part of plane commit, scalers will be committed
>    (i.e., either attached or detached) to respective planes in hw
>  - as part of crtc_commit, scaler will be either attached or detached
>    to crtc in hw
> 
> v2:
> -removed a log message (me)
> -changed input parameter to crtc_state (me)
> 
> Signed-off-by: Chandra Konduru <chandra.kond...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c |  138 
> +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h    |    3 +
>  2 files changed, 141 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 3903b90..407516a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -241,3 +241,141 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
>  {
>       drm_atomic_helper_crtc_destroy_state(crtc, state);
>  }
> +
> +/**
> + * intel_atomic_setup_scalers() - setup scalers for crtc per staged requests
> + * @dev: DRM device
> + * @crtc: intel crtc
> + * @crtc_state: incoming crtc_state to validate and setup scalers
> + *
> + * This function setups up scalers based on staged scaling requests for

s/setups up/sets up/

> + * a @crtc and its planes. It is called from crtc level check path. If 
> request
> + * is a supportable request, it attaches scalers to requested planes and 
> crtc.
> + *
> + * This function takes into account the current scaler(s) in use by any 
> planes
> + * not being part of this atomic state
> + *
> + *  Returns:
> + *         0 - scalers were setup succesfully
> + *         error code - otherwise
> + */
> +int intel_atomic_setup_scalers(struct drm_device *dev,
> +     struct intel_crtc *intel_crtc,
> +     struct intel_crtc_state *crtc_state)
> +{
> +     struct drm_plane *plane = NULL;
> +     struct intel_plane *intel_plane;
> +     struct intel_plane_state *plane_state = NULL;
> +     struct intel_crtc_scaler_state *scaler_state;
> +     struct drm_atomic_state *drm_state;
> +     int num_scalers_need;
> +     int i, j;
> +
> +     if (INTEL_INFO(dev)->gen < 9 || !intel_crtc || !crtc_state)
> +             return 0;
> +
> +     scaler_state = &crtc_state->scaler_state;
> +     drm_state = crtc_state->base.state;
> +
> +     num_scalers_need = hweight32(scaler_state->scaler_users);
> +     DRM_DEBUG_KMS("crtc_state = %p need = %d avail = %d scaler_users = 
> 0x%x\n",
> +             crtc_state, num_scalers_need, scaler_state->num_scalers,
> +             scaler_state->scaler_users);
> +
> +     /* if there is no change in scaler configuration, return */
> +     if (scaler_state->scaler_users ==
> +             intel_crtc->config->scaler_state.scaler_users)
> +             return 0;
> +
> +     /*
> +      * High level flow:
> +      * - staged scaler requests are already in scaler_state->scaler_users
> +      * - check whether staged scaling requests can be supported
> +      * - add planes using scalers that aren't in current transaction
> +      * - assign scalers to requested users
> +      * - as part of plane commit, scalers will be committed
> +      *   (i.e., either attached or detached) to respective planes in hw
> +      * - as part of crtc_commit, scaler will be either attached or detached
> +      *   to crtc in hw
> +      */
> +
> +     /* fail if required scalers > available scalers */
> +     if (num_scalers_need > scaler_state->num_scalers){
> +             DRM_DEBUG_KMS("Too many scaling requests %d > %d\n",
> +                     num_scalers_need, scaler_state->num_scalers);
> +             return -EINVAL;
> +     }
> +
> +     /* walkthrough scaler_users bits and start assigning scalers */
> +     for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) {
> +             int *scaler_id;
> +
> +             /* skip if scaler not required */
> +             if (!(scaler_state->scaler_users & (1 << i)))
> +                     continue;
> +
> +             if (i == SKL_CRTC_INDEX) {
> +                     /* panel fitter case: assign as a crtc scaler */
> +                     scaler_id = &scaler_state->scaler_id;
> +             } else {
> +                     if (!drm_state)
> +                             continue;

This shouldn't happen, at least not once we're back on the full atomic
helpers, right?  It seems like if it does happen, nothing's going to
work right, so maybe add a WARN_ON() to this test?

> +
> +                     /* plane scaler case: assign as a plane scaler */
> +                     /* find the plane that set the bit as scaler_user */
> +                     plane = drm_state->planes[i];
> +
> +                     /*
> +                      * to enable/disable hq mode, add planes that are using 
> scaler
> +                      * into this transaction
> +                      */
> +                     if (!plane) {
> +                             plane = drm_plane_from_index(dev, i);
> +                             drm_state->planes[i] = plane;
> +                             drm_state->plane_states[i] =
> +                                     drm_atomic_get_plane_state(drm_state, 
> plane);

I think you need to test for EDEADLK here in case we have a w/w
collision.  Other errors like ENOMEM might also happen.  Actually,
drm_atomic_get_plane_state() will take care of adding the state to the
plane_states[] array for you, so no need for the assignment; you can
just call the function and check for error returns.

> +                     }
> +
> +                     intel_plane = to_intel_plane(plane);
> +
> +                     /* plane on different crtc cannot be a scaler user of 
> this crtc */
> +                     if (WARN_ON(intel_plane->pipe != intel_crtc->pipe)) {
> +                             continue;
> +                     }
> +
> +                     plane_state = 
> to_intel_plane_state(drm_state->plane_states[i]);
> +                     scaler_id = &plane_state->scaler_id;
> +             }
> +
> +             if (*scaler_id < 0) {
> +                     /* find a free scaler */
> +                     for (j = 0; j < scaler_state->num_scalers; j++) {
> +                             if (!scaler_state->scalers[j].in_use) {
> +                                     scaler_state->scalers[j].in_use = 1;
> +                                     *scaler_id = 
> scaler_state->scalers[j].id;
> +                                     DRM_DEBUG_KMS("Attached scaler id %u.%u 
> to %s:%d\n",
> +                                             intel_crtc->pipe,
> +                                             i == SKL_CRTC_INDEX ? 
> scaler_state->scaler_id :
> +                                                     plane_state->scaler_id,
> +                                             i == SKL_CRTC_INDEX ? "CRTC" : 
> "PLANE",
> +                                             i == SKL_CRTC_INDEX ?  
> intel_crtc->base.base.id :
> +                                             plane->base.id);
> +                                     break;
> +                             }
> +                     }
> +             }
> +
> +             if (WARN_ON(*scaler_id < 0)) {
> +                     DRM_DEBUG_KMS("Cannot find scaler for %s:%d\n",
> +                             i == SKL_CRTC_INDEX ? "CRTC" : "PLANE",
> +                             i == SKL_CRTC_INDEX ? 
> intel_crtc->base.base.id:plane->base.id);
> +                     continue;
> +             }
> +
> +             /* set scaler mode */
> +             scaler_state->scalers[*scaler_id].mode = (num_scalers_need == 
> 1) ?
> +                     PS_SCALER_MODE_HQ : PS_SCALER_MODE_DYN;
> +     }
> +
> +     return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index f5d53c9..a9d787d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1401,6 +1401,9 @@ intel_atomic_get_crtc_state(struct drm_atomic_state 
> *state,
>  
>       return to_intel_crtc_state(crtc_state);
>  }
> +int intel_atomic_setup_scalers(struct drm_device *dev,
> +     struct intel_crtc *intel_crtc,
> +     struct intel_crtc_state *crtc_state);
>  
>  /* intel_atomic_plane.c */
>  struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
> -- 
> 1.7.9.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to