Hey,
Op 02-03-16 om 00:27 schreef Dhinakaran Pandiyan:
> From: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>
> Work around the CHV pipe C FIFO underruns that cause display failure by
> enabling sprite plane for cursor.
>
> This patch for BSW is based on Maarten Lankhorst's work that
> enables universal plane support.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Signed-off-by: Kalyan Kondapally <kalyan.kondapally@xxxxxxxxx>
> Signed-off-by: Pandiyan Dhinakaran <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 48 +++++++++++++++++++++++----------
> drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++--
> drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_sprite.c | 25 +++++++++---------
> 4 files changed, 100 insertions(+), 38 deletions(-)
Looks like CI is not unhappy with this patch, not sure I can review it since I'm the original author. :-)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 44fcff0..4a392d4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11941,9 +11941,14 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> bool is_crtc_enabled = crtc_state->active;
> bool turn_off, turn_on, visible, was_visible;
> struct drm_framebuffer *fb = plane_state->fb;
> + enum drm_plane_type plane_type = plane->type;
> +
> + if (plane_type == DRM_PLANE_TYPE_CURSOR &&
> + !pipe_has_cursor_plane(to_i915(dev), intel_crtc->pipe))
> + plane_type = DRM_PLANE_TYPE_OVERLAY;
>
> if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
> - plane->type != DRM_PLANE_TYPE_CURSOR) {
> + plane_type != DRM_PLANE_TYPE_CURSOR) {
> ret = skl_update_scaler_plane(
> to_intel_crtc_state(crtc_state),
> to_intel_plane_state(plane_state));
^Missing some plane->type -> plane_type in this function.
> <snip>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cb413e2..218f8f8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -971,9 +971,18 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
> return container_of(intel_hdmi, struct intel_digital_port, hdmi);
> }
>
> +static inline bool pipe_has_cursor_plane(struct drm_i915_private *dev_priv, int pipe)
> +{
> + if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C)
> + return false;
> +
> + return true;
> +}
> +
^Might be worth adding a comment why this is the case.
> /*
> * Returns the number of planes for this pipe, ie the number of sprites + 1
> - * (primary plane). This doesn't count the cursor plane then.
> + * (primary plane). This doesn't count the cursor plane except on CHV pipe C,
> + * which has a universal plane masked as cursor plane.
> */
> static inline unsigned int intel_num_planes(struct intel_crtc *crtc)
> {
> @@ -1595,7 +1604,8 @@ bool intel_sdvo_init(struct drm_device *dev,
>
>
> /* intel_sprite.c */
> -int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
> +struct drm_plane *intel_plane_init(struct drm_device *dev, enum pipe pipe,
> + int plane, enum drm_plane_type plane_type);
> int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> void intel_pipe_update_start(struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d33de95..0bab7d3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -941,7 +941,8 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
> if (WARN_ON(htotal == 0))
> htotal = 1;
>
> - if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
> + if (plane->base.type == DRM_PLANE_TYPE_CURSOR &&
> + pipe_has_cursor_plane(dev_priv, plane->pipe)) {
> /*
> * FIXME the formula gives values that are
> * too big for the cursor FIFO, and hence we
> @@ -970,7 +971,8 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
> struct intel_plane_state *state =
> to_intel_plane_state(plane->base.state);
>
> - if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
> + if (plane->base.type == DRM_PLANE_TYPE_CURSOR &&
> + pipe_has_cursor_plane(dev->dev_private, plane->pipe))
> continue;
>
> if (state->visible) {
> @@ -984,7 +986,8 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
> to_intel_plane_state(plane->base.state);
> unsigned int rate;
>
> - if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
> + if (plane->base.type == DRM_PLANE_TYPE_CURSOR &&
> + pipe_has_cursor_plane(dev->dev_private, plane->pipe)) {
> plane->wm.fifo_size = 63;
> continue;
> }
> @@ -1008,7 +1011,8 @@ static void vlv_compute_fifo(struct intel_crtc *crtc)
> if (fifo_left == 0)
> break;
>
> - if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
> + if (plane->base.type == DRM_PLANE_TYPE_CURSOR &&
> + pipe_has_cursor_plane(dev->dev_private, plane->pipe))
> continue;
>
> /* give it all to the first plane if none are active */
> @@ -1028,6 +1032,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
> {
> struct vlv_wm_state *wm_state = &crtc->wm_state;
> int level;
> + enum drm_plane_type plane_type;
>
> for (level = 0; level < wm_state->num_levels; level++) {
> struct drm_device *dev = crtc->base.dev;
> @@ -1038,7 +1043,13 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
> wm_state->sr[level].cursor = 63 - wm_state->sr[level].cursor;
>
> for_each_intel_plane_on_crtc(dev, crtc, plane) {
> - switch (plane->base.type) {
> + plane_type = plane->base.type;
> +
> + if (plane_type == DRM_PLANE_TYPE_CURSOR &&
> + !pipe_has_cursor_plane(to_i915(dev), plane->pipe))
> + plane_type = DRM_PLANE_TYPE_OVERLAY;
> +
> + switch (plane_type) {
> int sprite;
> case DRM_PLANE_TYPE_CURSOR:
> wm_state->wm[level].cursor = plane->wm.fifo_size -
> @@ -1065,6 +1076,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
> struct intel_plane *plane;
> int sr_fifo_size = INTEL_INFO(dev)->num_pipes * 512 - 1;
> int level;
> + enum drm_plane_type plane_type;
>
> memset(wm_state, 0, sizeof(*wm_state));
>
> @@ -1092,10 +1104,15 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
> if (!state->visible)
> continue;
>
> + plane_type = plane->base.type;
> + if (plane_type == DRM_PLANE_TYPE_CURSOR &&
> + !pipe_has_cursor_plane(to_i915(dev), plane->pipe))
> + plane_type = DRM_PLANE_TYPE_OVERLAY;
> +
> /* normal watermarks */
> for (level = 0; level < wm_state->num_levels; level++) {
> int wm = vlv_compute_wm_level(plane, crtc, state, level);
> - int max_wm = plane->base.type == DRM_PLANE_TYPE_CURSOR ? 63 : 511;
> + int max_wm = plane_type == DRM_PLANE_TYPE_CURSOR ? 63 : 511;
>
> /* hack */
> if (WARN_ON(level == 0 && wm > max_wm))
> @@ -1104,7 +1121,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
> if (wm > plane->wm.fifo_size)
> break;
>
> - switch (plane->base.type) {
> + switch (plane_type) {
> int sprite;
> case DRM_PLANE_TYPE_CURSOR:
> wm_state->wm[level].cursor = wm;
> @@ -1125,7 +1142,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
> continue;
>
> /* maxfifo watermarks */
> - switch (plane->base.type) {
> + switch (plane_type) {
> int sprite, level;
> case DRM_PLANE_TYPE_CURSOR:
> for (level = 0; level < wm_state->num_levels; level++)
> @@ -1166,9 +1183,16 @@ static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc)
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_plane *plane;
> int sprite0_start = 0, sprite1_start = 0, fifo_size = 0;
> + enum drm_plane_type plane_type;
>
> for_each_intel_plane_on_crtc(dev, crtc, plane) {
> - if (plane->base.type == DRM_PLANE_TYPE_CURSOR) {
> + plane_type = plane->base.type;
> +
> + if (plane_type == DRM_PLANE_TYPE_CURSOR &&
> + !pipe_has_cursor_plane(to_i915(dev), plane->pipe))
> + plane_type = DRM_PLANE_TYPE_OVERLAY;
if (plane->type == CURSOR && pipe_has_cursor_plane) { WARN_ON(..); continue; } ? Nothing else needs plane_type here.
I can't comment on the watermark changes, but if CI is happy so am I.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
