On Tue, Mar 01, 2016 at 03:27:17PM -0800, Dhinakaran Pandiyan wrote:
> 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(-)
>
> 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;
I think spreading this stuff all over is too messy. I would suggest adding some
kind of hw plane type to intel_plane instead.
>
> 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));
> @@ -14472,7 +14477,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> struct intel_crtc_state *crtc_state = NULL;
> struct drm_plane *primary = NULL;
> struct drm_plane *cursor = NULL;
> - int i, ret;
> + int i, ret, sprite;
>
> intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
> if (intel_crtc == NULL)
> @@ -14499,9 +14504,32 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> if (!primary)
> goto fail;
>
> - cursor = intel_cursor_plane_create(dev, pipe);
> - if (!cursor)
> - goto fail;
> + if (pipe_has_cursor_plane(dev_priv, pipe)) {
> + cursor = intel_cursor_plane_create(dev, pipe);
> + if (!cursor)
> + goto fail;
> + }
> +
> + for_each_sprite(dev_priv, pipe, sprite) {
> + enum drm_plane_type plane_type;
> + struct drm_plane *plane;
> +
> + if (sprite + 1 < INTEL_INFO(dev_priv)->num_sprites[pipe] ||
> + pipe_has_cursor_plane(dev_priv, pipe))
> + plane_type = DRM_PLANE_TYPE_OVERLAY;
> + else
> + plane_type = DRM_PLANE_TYPE_CURSOR;
I'm thinking I might put this logic into the sprite init code.
Apart from these the main concern I have with all this is watermarks, cxsr,
vblank waits etc. I can't see anything here to explain how it's going to
actually work so I suspect there will be problems.
> +
> + plane = intel_plane_init(dev, pipe, sprite, plane_type);
> + if (IS_ERR(plane)) {
> + DRM_DEBUG_KMS("pipe %c sprite %c init failed: %ld\n",
> + pipe_name(pipe), sprite_name(pipe, sprite), PTR_ERR(plane));
> + goto fail;
> + }
> +
> + if (plane_type == DRM_PLANE_TYPE_CURSOR)
> + cursor = plane;
> + }
>
> ret = drm_crtc_init_with_planes(dev, &intel_crtc->base, primary,
> cursor, &intel_crtc_funcs, NULL);
> @@ -15534,7 +15562,6 @@ fail:
> void intel_modeset_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int sprite, ret;
> enum pipe pipe;
> struct intel_crtc *crtc;
>
> @@ -15606,15 +15633,8 @@ void intel_modeset_init(struct drm_device *dev)
> INTEL_INFO(dev)->num_pipes,
> INTEL_INFO(dev)->num_pipes > 1 ? "s" : "");
>
> - for_each_pipe(dev_priv, pipe) {
> + for_each_pipe(dev_priv, pipe)
> intel_crtc_init(dev, pipe);
> - for_each_sprite(dev_priv, pipe, sprite) {
> - ret = intel_plane_init(dev, pipe, sprite);
> - if (ret)
> - DRM_DEBUG_KMS("pipe %c sprite %c init failed: %d\n",
> - pipe_name(pipe), sprite_name(pipe, sprite), ret);
> - }
> - }
>
> intel_update_czclk(dev_priv);
> intel_update_cdclk(dev);
> 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;
> +}
> +
> /*
> * 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 == DRM_PLANE_TYPE_CURSOR) {
> WARN_ON(plane->wm.fifo_size != 63);
> continue;
> }
> @@ -3996,11 +4020,18 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
> struct intel_plane *plane;
> enum pipe pipe;
> u32 val;
> + enum drm_plane_type plane_type;
>
> vlv_read_wm_values(dev_priv, wm);
>
> for_each_intel_plane(dev, 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:
> plane->wm.fifo_size = 63;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 8821533..0b5104c 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1022,8 +1022,8 @@ static uint32_t skl_plane_formats[] = {
> DRM_FORMAT_VYUY,
> };
>
> -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)
> {
> struct intel_plane *intel_plane;
> struct intel_plane_state *state;
> @@ -1033,16 +1033,16 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> int ret;
>
> if (INTEL_INFO(dev)->gen < 5)
> - return -ENODEV;
> + return ERR_PTR(-ENODEV);
>
> intel_plane = kzalloc(sizeof(*intel_plane), GFP_KERNEL);
> if (!intel_plane)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> state = intel_create_plane_state(&intel_plane->base);
> if (!state) {
> kfree(intel_plane);
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
> }
> intel_plane->base.state = &state->base;
>
> @@ -1097,8 +1097,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> break;
> default:
> - kfree(intel_plane);
> - return -ENODEV;
> + ret = -ENODEV;
> + goto out;
> }
>
> intel_plane->pipe = pipe;
> @@ -1109,16 +1109,17 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs,
> &intel_plane_funcs,
> plane_formats, num_plane_formats,
> - DRM_PLANE_TYPE_OVERLAY, NULL);
> - if (ret) {
> - kfree(intel_plane);
> + plane_type, NULL);
> + if (ret)
> goto out;
> - }
>
> intel_create_rotation_property(dev, intel_plane);
>
> drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
> + return &intel_plane->base;
>
> out:
> - return ret;
> + kfree(intel_plane);
> + kfree(state);
> + return ERR_PTR(ret);
> }
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
