| Message ID | 1426896282-23215-10-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.konduru@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 >
> -----Original Message----- > From: Roper, Matthew D > Sent: Tuesday, March 24, 2015 10:15 PM > To: Konduru, Chandra > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander > Subject: Re: [PATCH 09/21 v2] drm/i915: Add atomic function to setup scalers > scalers for a crtc. > > 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.konduru@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/ Yes, will update. > > > + * 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? Two reasons for having above check: 1) Yes, that is what expected in case of planes. But because current tree is not having full atomic helpers, placed this check. 2) During legacy setmode requiring/freeing panel fitter, it can be null. To cover those cases, placed this check for proper operation of scaler for panel fitter. > > > + > > + /* 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. Ok, will return in case of error from drm_atomic_get_plane_state(). Will send updated patch shortly. > > > + } > > + > > + 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
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 + * 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; + + /* 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); + } + + 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);
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.konduru@intel.com> --- drivers/gpu/drm/i915/intel_atomic.c | 138 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 3 + 2 files changed, 141 insertions(+)