| Message ID | 1426896282-23215-9-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 20, 2015 at 05:04:29PM -0700, Chandra Konduru wrote: > This helper function stages a scaler request for a plane/crtc into > crtc_state->scaler_users (which is a bit field). It also performs > required checks before staging any change into scaler_state. > > v2: > -updates to use single copy of scaler limits (Matt) > -added force detach parameter for pfit disable purpose (me) > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 143 ++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 + > 2 files changed, 146 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 890d372..976bfb1 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12387,6 +12387,149 @@ intel_cleanup_plane_fb(struct drm_plane *plane, > } > } > > +/* > + * Update crtc_state->scaler_users for requested plane or crtc based on state. I realize you're trying to re-use some logic here, but I find the overloaded function that works on either planes or crtcs a bit harder to follow. Personally I'd prefer if this were just two separate functions, one focused on planes, one focused on CRTC's, even if it means duplicating a little bit of the logic. > + * > + * plane (in) > + * NULL - for crtc > + * not NULL - for plane > + * force_detach (in) > + * unconditionally scaler will be staged for detachment from crtc/plane > + * Return > + * 0 - scaler_usage updated successfully > + * error - requested scaling cannot be supported or other error condition > + */ Probably want to put this in kerneldoc format. > +int > +skl_update_scaler_users( > + struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state, > + struct intel_plane *intel_plane, struct intel_plane_state *plane_state, Might be a little easier to drop the intel_crtc and intel_plane parameters and just pull them out of the state backpointers. > + int force_detach) > +{ > + int need_scaling; > + int idx; > + int src_w, src_h, dst_w, dst_h; > + int scaler_id; > + struct drm_framebuffer *fb; > + struct intel_crtc_scaler_state *scaler_state; > + > + if (!intel_crtc || !crtc_state || > + (intel_plane && intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)) > + return 0; > + > + scaler_state = &crtc_state->scaler_state; > + > + if (!scaler_state->num_scalers) { > + DRM_DEBUG_KMS("crtc_state = %p, num_scalers = %d\n", crtc_state, > + scaler_state->num_scalers); > + return 0; > + } > + > + idx = intel_plane ? drm_plane_index(&intel_plane->base) : SKL_CRTC_INDEX; > + fb = intel_plane ? plane_state->base.fb : NULL; > + > + if (intel_plane) { > + src_w = drm_rect_width(&plane_state->src) >> 16; > + src_h = drm_rect_height(&plane_state->src) >> 16; > + dst_w = drm_rect_width(&plane_state->dst); > + dst_h = drm_rect_height(&plane_state->dst); > + scaler_id = plane_state->scaler_id; > + } else { > + struct drm_display_mode *adjusted_mode = > + &crtc_state->base.adjusted_mode; > + src_w = crtc_state->pipe_src_w; > + src_h = crtc_state->pipe_src_h; > + dst_w = adjusted_mode->hdisplay; > + dst_h = adjusted_mode->vdisplay; > + scaler_id = scaler_state->scaler_id; > + } > + need_scaling = (src_w != dst_w || src_h != dst_h); We're already calculating this in the plane check function; we should probably just store the result in a plane_state field at that point so we don't have to re-calculate it here. > + > + /* > + * if plane is being disabled or scaler is no more required or force detach Maybe it will become more clear later in the patch series, but I'm not sure I understand what 'force_detach' does. Won't scalers that we don't end up re-assigning automatically be cleared when we nobody makes use of them in a new commit? > + * - free scaler binded to this plane/crtc > + * - in order to do this, update crtc->scaler_usage > + * > + * Here scaler state in crtc_state is set free so that > + * scaler can be assigned to other user. Actual register > + * update to free the scaler is done in plane/panel-fit programming. > + * For this purpose crtc/plane_state->scaler_id isn't reset here. > + */ > + if (force_detach || !need_scaling || (intel_plane && > + (!fb || !plane_state->visible))) { > + if (scaler_id >= 0) { > + scaler_state->scaler_users &= ~(1 << idx); > + scaler_state->scalers[scaler_id].in_use = 0; > + > + DRM_DEBUG_KMS("Staged freeing scaler id %u.%u from %s:%d " > + "crtc_state = %p scaler_users = 0x%x\n", > + intel_crtc->pipe, scaler_id, intel_plane ? "PLANE" : "CRTC", > + intel_plane ? intel_plane->base.base.id : > + intel_crtc->base.base.id, crtc_state, > + scaler_state->scaler_users); > + } > + return 0; > + } > + > + /* > + * check for rect size: > + * min sizes in case of scaling involved > + * max sizes in all cases > + */ > + if ((need_scaling && need_scaling is guaranteed to be true now (otherwise we would have returned above), so we can drop this test. > + (src_w < scaler_state->min_src_w || src_h < scaler_state->min_src_h || > + dst_w < scaler_state->min_dst_w || dst_h < scaler_state->min_dst_h)) || > + > + src_w > scaler_state->max_src_w || src_h > scaler_state->max_src_h || > + dst_w > scaler_state->max_dst_w || dst_h > scaler_state->max_dst_h) { > + DRM_DEBUG_KMS("%s:%d scaler_user index %u.%u: src %ux%u dst %ux%u " > + "size is out of scaler range\n", > + intel_plane ? "PLANE" : "CRTC", > + intel_plane ? intel_plane->base.base.id : intel_crtc->base.base.id, > + intel_crtc->pipe, idx, src_w, src_h, dst_w, dst_h); > + return -EINVAL; > + } > + > + /* check colorkey */ > + if (intel_plane && need_scaling && intel_colorkey_enabled(intel_plane)) { Can drop 'need_scaling' test again. > + DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed", > + intel_plane->base.base.id); > + return -EINVAL; > + } > + > + /* Check src format */ > + if (intel_plane && need_scaling) { Ditto > + switch (fb->pixel_format) { > + case DRM_FORMAT_RGB565: > + case DRM_FORMAT_XBGR8888: > + case DRM_FORMAT_XRGB8888: > + case DRM_FORMAT_ABGR8888: > + case DRM_FORMAT_ARGB8888: > + case DRM_FORMAT_XRGB2101010: > + case DRM_FORMAT_ARGB2101010: > + case DRM_FORMAT_XBGR2101010: > + case DRM_FORMAT_ABGR2101010: > + case DRM_FORMAT_YUYV: > + case DRM_FORMAT_YVYU: > + case DRM_FORMAT_UYVY: > + case DRM_FORMAT_VYUY: > + break; > + default: > + DRM_DEBUG_KMS("PLANE:%d FB:%d format 0x%x not supported\n", > + intel_plane->base.base.id, fb->base.id, fb->pixel_format); Might want to add "...while scaling" to this message to avoid confusion. > + return -EINVAL; > + } > + } > + > + /* mark this plane as a scaler user in crtc_state */ > + scaler_state->scaler_users |= (1 << idx); > + DRM_DEBUG_KMS("%s:%d staged scaling request for %ux%u->%ux%u " > + "crtc_state = %p scaler_users = 0x%x\n", > + intel_plane ? "PLANE" : "CRTC", > + intel_plane ? intel_plane->base.base.id : intel_crtc->base.base.id, > + src_w, src_h, dst_w, dst_h, crtc_state, scaler_state->scaler_users); > + return 0; > +} > + > static int > intel_check_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1da5087..f5d53c9 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1138,6 +1138,9 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, > struct intel_crtc_state *pipe_config); > void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc); > void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); > +int skl_update_scaler_users(struct intel_crtc *intel_crtc, > + struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane, > + struct intel_plane_state *plane_state, int force_detach); > > /* intel_dp.c */ > void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); > -- > 1.7.9.5 You should probably just squash this into the later patches where you actually call it (e.g., patch 18) so it's more clear how/when/where it is used. Matt >
> -----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 08/21 v2] drm/i915: Add helper function to update > scaler_users in crtc_state > > On Fri, Mar 20, 2015 at 05:04:29PM -0700, Chandra Konduru wrote: > > This helper function stages a scaler request for a plane/crtc into > > crtc_state->scaler_users (which is a bit field). It also performs > > required checks before staging any change into scaler_state. > > > > v2: > > -updates to use single copy of scaler limits (Matt) -added force > > detach parameter for pfit disable purpose (me) > > > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 143 > ++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 + > > 2 files changed, 146 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 890d372..976bfb1 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -12387,6 +12387,149 @@ intel_cleanup_plane_fb(struct drm_plane > *plane, > > } > > } > > > > +/* > > + * Update crtc_state->scaler_users for requested plane or crtc based on > state. > > I realize you're trying to re-use some logic here, but I find the overloaded > function that works on either planes or crtcs a bit harder to follow. Personally > I'd prefer if this were just two separate functions, one focused on planes, one > focused on CRTC's, even if it means duplicating a little bit of the logic. > Yes, this is written as per one of the earlier feedback comment. I know this is bit complex, but I find it is worth to take care of all Checks in one place covering both crtc and plane. To avoid any confusion, I have put sufficient commentary throughout the code. let me know if you think any further commentary is required. > > + * > > + * plane (in) > > + * NULL - for crtc > > + * not NULL - for plane > > + * force_detach (in) > > + * unconditionally scaler will be staged for detachment from crtc/plane > > + * Return > > + * 0 - scaler_usage updated successfully > > + * error - requested scaling cannot be supported or other error condition > > + */ > > Probably want to put this in kerneldoc format. Will update in next rev. > > > +int > > +skl_update_scaler_users( > > + struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state, > > + struct intel_plane *intel_plane, struct intel_plane_state > > +*plane_state, > > Might be a little easier to drop the intel_crtc and intel_plane parameters and just > pull them out of the state backpointers. You know there is a check caller is doing to get right crtc pointer in case of check_plane: crtc = crtc ? crtc : plane->crtc; If crtc isn't passed, then similar check is required in this function. As caller is already taking care of this, passing crtc just to avoid another check. > > > + int force_detach) > > +{ > > + int need_scaling; > > + int idx; > > + int src_w, src_h, dst_w, dst_h; > > + int scaler_id; > > + struct drm_framebuffer *fb; > > + struct intel_crtc_scaler_state *scaler_state; > > + > > + if (!intel_crtc || !crtc_state || > > + (intel_plane && intel_plane->base.type == > DRM_PLANE_TYPE_CURSOR)) > > + return 0; > > + > > + scaler_state = &crtc_state->scaler_state; > > + > > + if (!scaler_state->num_scalers) { > > + DRM_DEBUG_KMS("crtc_state = %p, num_scalers = %d\n", > crtc_state, > > + scaler_state->num_scalers); > > + return 0; > > + } > > + > > + idx = intel_plane ? drm_plane_index(&intel_plane->base) : > SKL_CRTC_INDEX; > > + fb = intel_plane ? plane_state->base.fb : NULL; > > + > > + if (intel_plane) { > > + src_w = drm_rect_width(&plane_state->src) >> 16; > > + src_h = drm_rect_height(&plane_state->src) >> 16; > > + dst_w = drm_rect_width(&plane_state->dst); > > + dst_h = drm_rect_height(&plane_state->dst); > > + scaler_id = plane_state->scaler_id; > > + } else { > > + struct drm_display_mode *adjusted_mode = > > + &crtc_state->base.adjusted_mode; > > + src_w = crtc_state->pipe_src_w; > > + src_h = crtc_state->pipe_src_h; > > + dst_w = adjusted_mode->hdisplay; > > + dst_h = adjusted_mode->vdisplay; > > + scaler_id = scaler_state->scaler_id; > > + } > > + need_scaling = (src_w != dst_w || src_h != dst_h); > > We're already calculating this in the plane check function; we should probably > just store the result in a plane_state field at that point so we don't have to re- > calculate it here. I think you are referring to sprite plane check, but this is not there in primary plane check function. > > > > + > > + /* > > + * if plane is being disabled or scaler is no more required or force > > +detach > > Maybe it will become more clear later in the patch series, but I'm not sure I > understand what 'force_detach' does. Won't scalers that we don't end up re- > assigning automatically be cleared when we nobody makes use of them in a new > commit? Force detach, as name suggests, it stages force freeing of a scaler if one is attached to crtc or plane. This is used when disabling panel fitter which is called from crtc disable. > > > + * - free scaler binded to this plane/crtc > > + * - in order to do this, update crtc->scaler_usage > > + * > > + * Here scaler state in crtc_state is set free so that > > + * scaler can be assigned to other user. Actual register > > + * update to free the scaler is done in plane/panel-fit programming. > > + * For this purpose crtc/plane_state->scaler_id isn't reset here. > > + */ > > + if (force_detach || !need_scaling || (intel_plane && > > + (!fb || !plane_state->visible))) { > > + if (scaler_id >= 0) { > > + scaler_state->scaler_users &= ~(1 << idx); > > + scaler_state->scalers[scaler_id].in_use = 0; > > + > > + DRM_DEBUG_KMS("Staged freeing scaler id %u.%u > from %s:%d " > > + "crtc_state = %p scaler_users = 0x%x\n", > > + intel_crtc->pipe, scaler_id, intel_plane ? > "PLANE" : "CRTC", > > + intel_plane ? intel_plane->base.base.id : > > + intel_crtc->base.base.id, crtc_state, > > + scaler_state->scaler_users); > > + } > > + return 0; > > + } > > + > > + /* > > + * check for rect size: > > + * min sizes in case of scaling involved > > + * max sizes in all cases > > + */ > > + if ((need_scaling && > > need_scaling is guaranteed to be true now (otherwise we would have returned > above), so we can drop this test. > > > + (src_w < scaler_state->min_src_w || src_h < scaler_state- > >min_src_h || > > + dst_w < scaler_state->min_dst_w || dst_h < > > +scaler_state->min_dst_h)) || > > + > > + src_w > scaler_state->max_src_w || src_h > scaler_state- > >max_src_h || > > + dst_w > scaler_state->max_dst_w || dst_h > scaler_state- > >max_dst_h) { > > + DRM_DEBUG_KMS("%s:%d scaler_user index %u.%u: src %ux%u > dst %ux%u " > > + "size is out of scaler range\n", > > + intel_plane ? "PLANE" : "CRTC", > > + intel_plane ? intel_plane->base.base.id : intel_crtc- > >base.base.id, > > + intel_crtc->pipe, idx, src_w, src_h, dst_w, dst_h); > > + return -EINVAL; > > + } > > + > > + /* check colorkey */ > > + if (intel_plane && need_scaling && > > +intel_colorkey_enabled(intel_plane)) { > > Can drop 'need_scaling' test again. > > > + DRM_DEBUG_KMS("PLANE:%d scaling with color key not > allowed", > > + intel_plane->base.base.id); > > + return -EINVAL; > > + } > > + > > + /* Check src format */ > > + if (intel_plane && need_scaling) { > > Ditto > > > + switch (fb->pixel_format) { > > + case DRM_FORMAT_RGB565: > > + case DRM_FORMAT_XBGR8888: > > + case DRM_FORMAT_XRGB8888: > > + case DRM_FORMAT_ABGR8888: > > + case DRM_FORMAT_ARGB8888: > > + case DRM_FORMAT_XRGB2101010: > > + case DRM_FORMAT_ARGB2101010: > > + case DRM_FORMAT_XBGR2101010: > > + case DRM_FORMAT_ABGR2101010: > > + case DRM_FORMAT_YUYV: > > + case DRM_FORMAT_YVYU: > > + case DRM_FORMAT_UYVY: > > + case DRM_FORMAT_VYUY: > > + break; > > + default: > > + DRM_DEBUG_KMS("PLANE:%d FB:%d format 0x%x not > supported\n", > > + intel_plane->base.base.id, fb->base.id, fb- > >pixel_format); > > Might want to add "...while scaling" to this message to avoid confusion. > > > + return -EINVAL; > > + } > > + } > > + > > + /* mark this plane as a scaler user in crtc_state */ > > + scaler_state->scaler_users |= (1 << idx); > > + DRM_DEBUG_KMS("%s:%d staged scaling request for %ux%u->%ux%u " > > + "crtc_state = %p scaler_users = 0x%x\n", > > + intel_plane ? "PLANE" : "CRTC", > > + intel_plane ? intel_plane->base.base.id : intel_crtc- > >base.base.id, > > + src_w, src_h, dst_w, dst_h, crtc_state, scaler_state- > >scaler_users); > > + return 0; > > +} > > + > > static int > > intel_check_primary_plane(struct drm_plane *plane, > > struct intel_plane_state *state) diff --git > > a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 1da5087..f5d53c9 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1138,6 +1138,9 @@ void intel_mode_from_pipe_config(struct > drm_display_mode *mode, > > struct intel_crtc_state *pipe_config); void > > intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc); void > > intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); > > +int skl_update_scaler_users(struct intel_crtc *intel_crtc, > > + struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane, > > + struct intel_plane_state *plane_state, int force_detach); > > > > /* intel_dp.c */ > > void intel_dp_init(struct drm_device *dev, int output_reg, enum port > > port); > > -- > > 1.7.9.5 > > You should probably just squash this into the later patches where you actually > call it (e.g., patch 18) so it's more clear how/when/where it is used. Same point came up during review of previous version, and for now it is agreed to keep current split to avoid redo of the whole series. I hope you are ok with this. > > > Matt > > > > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
On Wed, Mar 25, 2015 at 12:20:08PM -0700, Konduru, Chandra wrote: ... > > > > We're already calculating this in the plane check function; we should probably > > just store the result in a plane_state field at that point so we don't have to re- > > calculate it here. > > I think you are referring to sprite plane check, but this is not there in primary plane > check function. Well, it is, but it's hidden inside the drm plane helper function and isn't directly in our i915 driver code. I feel like "do these src/dest rectangles imply scaling?" is a question that is going to be relevant to all drivers, not just i915, so it might even be worth adding the field to the DRM core's state structure and have the helper function set it for any drivers using the helper. Hardware drivers could override this value, or could set it themselves if they don't use the helper. Just a thought... Matt
> -----Original Message----- > From: Roper, Matthew D > Sent: Wednesday, March 25, 2015 1:59 PM > To: Konduru, Chandra > Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander > Subject: Re: [PATCH 08/21 v2] drm/i915: Add helper function to update > scaler_users in crtc_state > > On Wed, Mar 25, 2015 at 12:20:08PM -0700, Konduru, Chandra wrote: > ... > > > > > > We're already calculating this in the plane check function; we > > > should probably just store the result in a plane_state field at that > > > point so we don't have to re- calculate it here. > > > > I think you are referring to sprite plane check, but this is not there > > in primary plane check function. > > Well, it is, but it's hidden inside the drm plane helper function and isn't directly in > our i915 driver code. > > I feel like "do these src/dest rectangles imply scaling?" is a question that is going > to be relevant to all drivers, not just i915, so it might even be worth adding the > field to the DRM core's state structure and have the helper function set it for any > drivers using the helper. > Hardware drivers could override this value, or could set it themselves if they > don't use the helper. Just a thought... I think that makes sense, but not planning in this patch series. > > > Matt > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 890d372..976bfb1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12387,6 +12387,149 @@ intel_cleanup_plane_fb(struct drm_plane *plane, } } +/* + * Update crtc_state->scaler_users for requested plane or crtc based on state. + * + * plane (in) + * NULL - for crtc + * not NULL - for plane + * force_detach (in) + * unconditionally scaler will be staged for detachment from crtc/plane + * Return + * 0 - scaler_usage updated successfully + * error - requested scaling cannot be supported or other error condition + */ +int +skl_update_scaler_users( + struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state, + struct intel_plane *intel_plane, struct intel_plane_state *plane_state, + int force_detach) +{ + int need_scaling; + int idx; + int src_w, src_h, dst_w, dst_h; + int scaler_id; + struct drm_framebuffer *fb; + struct intel_crtc_scaler_state *scaler_state; + + if (!intel_crtc || !crtc_state || + (intel_plane && intel_plane->base.type == DRM_PLANE_TYPE_CURSOR)) + return 0; + + scaler_state = &crtc_state->scaler_state; + + if (!scaler_state->num_scalers) { + DRM_DEBUG_KMS("crtc_state = %p, num_scalers = %d\n", crtc_state, + scaler_state->num_scalers); + return 0; + } + + idx = intel_plane ? drm_plane_index(&intel_plane->base) : SKL_CRTC_INDEX; + fb = intel_plane ? plane_state->base.fb : NULL; + + if (intel_plane) { + src_w = drm_rect_width(&plane_state->src) >> 16; + src_h = drm_rect_height(&plane_state->src) >> 16; + dst_w = drm_rect_width(&plane_state->dst); + dst_h = drm_rect_height(&plane_state->dst); + scaler_id = plane_state->scaler_id; + } else { + struct drm_display_mode *adjusted_mode = + &crtc_state->base.adjusted_mode; + src_w = crtc_state->pipe_src_w; + src_h = crtc_state->pipe_src_h; + dst_w = adjusted_mode->hdisplay; + dst_h = adjusted_mode->vdisplay; + scaler_id = scaler_state->scaler_id; + } + need_scaling = (src_w != dst_w || src_h != dst_h); + + /* + * if plane is being disabled or scaler is no more required or force detach + * - free scaler binded to this plane/crtc + * - in order to do this, update crtc->scaler_usage + * + * Here scaler state in crtc_state is set free so that + * scaler can be assigned to other user. Actual register + * update to free the scaler is done in plane/panel-fit programming. + * For this purpose crtc/plane_state->scaler_id isn't reset here. + */ + if (force_detach || !need_scaling || (intel_plane && + (!fb || !plane_state->visible))) { + if (scaler_id >= 0) { + scaler_state->scaler_users &= ~(1 << idx); + scaler_state->scalers[scaler_id].in_use = 0; + + DRM_DEBUG_KMS("Staged freeing scaler id %u.%u from %s:%d " + "crtc_state = %p scaler_users = 0x%x\n", + intel_crtc->pipe, scaler_id, intel_plane ? "PLANE" : "CRTC", + intel_plane ? intel_plane->base.base.id : + intel_crtc->base.base.id, crtc_state, + scaler_state->scaler_users); + } + return 0; + } + + /* + * check for rect size: + * min sizes in case of scaling involved + * max sizes in all cases + */ + if ((need_scaling && + (src_w < scaler_state->min_src_w || src_h < scaler_state->min_src_h || + dst_w < scaler_state->min_dst_w || dst_h < scaler_state->min_dst_h)) || + + src_w > scaler_state->max_src_w || src_h > scaler_state->max_src_h || + dst_w > scaler_state->max_dst_w || dst_h > scaler_state->max_dst_h) { + DRM_DEBUG_KMS("%s:%d scaler_user index %u.%u: src %ux%u dst %ux%u " + "size is out of scaler range\n", + intel_plane ? "PLANE" : "CRTC", + intel_plane ? intel_plane->base.base.id : intel_crtc->base.base.id, + intel_crtc->pipe, idx, src_w, src_h, dst_w, dst_h); + return -EINVAL; + } + + /* check colorkey */ + if (intel_plane && need_scaling && intel_colorkey_enabled(intel_plane)) { + DRM_DEBUG_KMS("PLANE:%d scaling with color key not allowed", + intel_plane->base.base.id); + return -EINVAL; + } + + /* Check src format */ + if (intel_plane && need_scaling) { + switch (fb->pixel_format) { + case DRM_FORMAT_RGB565: + case DRM_FORMAT_XBGR8888: + case DRM_FORMAT_XRGB8888: + case DRM_FORMAT_ABGR8888: + case DRM_FORMAT_ARGB8888: + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_ARGB2101010: + case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_ABGR2101010: + case DRM_FORMAT_YUYV: + case DRM_FORMAT_YVYU: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_VYUY: + break; + default: + DRM_DEBUG_KMS("PLANE:%d FB:%d format 0x%x not supported\n", + intel_plane->base.base.id, fb->base.id, fb->pixel_format); + return -EINVAL; + } + } + + /* mark this plane as a scaler user in crtc_state */ + scaler_state->scaler_users |= (1 << idx); + DRM_DEBUG_KMS("%s:%d staged scaling request for %ux%u->%ux%u " + "crtc_state = %p scaler_users = 0x%x\n", + intel_plane ? "PLANE" : "CRTC", + intel_plane ? intel_plane->base.base.id : intel_crtc->base.base.id, + src_w, src_h, dst_w, dst_h, crtc_state, scaler_state->scaler_users); + return 0; +} + static int intel_check_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1da5087..f5d53c9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1138,6 +1138,9 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, struct intel_crtc_state *pipe_config); void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc); void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); +int skl_update_scaler_users(struct intel_crtc *intel_crtc, + struct intel_crtc_state *crtc_state, struct intel_plane *intel_plane, + struct intel_plane_state *plane_state, int force_detach); /* intel_dp.c */ void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
This helper function stages a scaler request for a plane/crtc into crtc_state->scaler_users (which is a bit field). It also performs required checks before staging any change into scaler_state. v2: -updates to use single copy of scaler limits (Matt) -added force detach parameter for pfit disable purpose (me) Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 143 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 3 + 2 files changed, 146 insertions(+)