Re: [Intel-gfx] [PATCH] drm/i915: Don't try to handle GuC when GuC is not supported.

[Paulo Zanoni]

Em Qua, 2016-09-21 às 11:22 -0700, Rodrigo Vivi escreveu:
> Avoid any kind of GuC handling if GuC is not supported
> on a giving platform.
> 
> Besides being useless handling, our driver needs
> to be smarter than the user trying to use an invalid paramenter.

So the problem is when a platform doesn't support guc and the user
passes i915.enable_guc_something=1, right?

> 
> Cc: Jani Nikula 
> Cc: Anusha Srivatsa 
> Cc: Christophe Prigent 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97573
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 6fd39ef..da0f5ed 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -720,6 +720,13 @@ void intel_guc_init(struct drm_device *dev)
>   struct intel_guc_fw *guc_fw = _priv->guc.guc_fw;
>   const char *fw_path;
>  
> + if (!HAS_GUC(dev)) {
> + i915.enable_guc_loading = 0;
> + i915.enable_guc_submission = 0;
> + fw_path = NULL;
> + return;
> + }

Instead of this, how about we just patch the code below with:

if (!HAS_GUC(dev_priv)) {
i915.enable_guc_loading = 0;
i915.enable_guc_submission = 0;
} else {
/* A negative value means "use platform default" */
if (i915.enable_guc_loading < 0)
i915.enable_guc_loading = HAS_GUC_UCODE(dev_priv);
if (i915.enable_guc_submission < 0)
i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
}

Or we could even go with our current "design pattern" and create
intel_sanitize_guc_options().

This way we'll be able to avoid adding a second failure code path,
since we already have one for platforms with guc but options disabled.


> +
>   /* A negative value means "use platform default" */
>   if (i915.enable_guc_loading < 0)
>   i915.enable_guc_loading = HAS_GUC_UCODE(dev);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4] drm/i915/bxt: Implement Transition WM

[Paulo Zanoni]

Em Qua, 2016-09-14 às 17:24 +0530, Kumar, Mahesh escreveu:
> From: Mahesh Kumar 
> 
> This patch enables Transition WM for SKL+ platforms.
> 
> Transition WM are used if IPC is enabled, to decide, number of blocks
> to be fetched before reducing the priority of display to fetch from
> memory.
> 
> Changes since v1:
>  - Don't enable transition WM for GEN9 (Art/Paulo)
>  - Rebase to use fixed point 16.16 calculation
>  - Fix the use of selected result from latency level-0
> 
> Changes since v2:
>  - Fix transition WM enable condition
> 
> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 60
> ++---
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index de2e738..4814a1a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2862,6 +2862,8 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
>  
>  #define SKL_DDB_SIZE 896 /* in blocks */
>  #define BXT_DDB_SIZE 512
> +#define SKL_TRANS_WM_AMOUNT  10  /* tunable value */
> +#define SKL_TRANS_WM_MIN 14
>  #define SKL_SAGV_BLOCK_TIME  30 /* µs */
>  
>  /*
> @@ -3583,6 +3585,48 @@ static uint32_t
> skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cst
>   return pixel_rate;
>  }
>  
> +static void skl_compute_plane_trans_wm(const struct drm_i915_private
> *dev_priv,
> + struct skl_pipe_wm *pipe_wm,
> + struct intel_plane_state
> *intel_pstate,
> + uint32_t selected_result,
> + uint32_t y_tile_min,
> + bool y_tile)
> +{
> + struct drm_plane_state *pstate = _pstate->base;
> + int id = skl_wm_plane_id(to_intel_plane(pstate->plane));
> + uint16_t *out_blocks = _wm->trans_wm.plane_res_b[id];
> + uint8_t *out_lines = _wm->trans_wm.plane_res_l[id];
> + uint32_t trans_min = 0, trans_offset_blocks;
> + uint32_t trans_y_tile_min = 0, res_blocks = 0;
> + uint16_t trans_res_blocks = 0;
> +
> + /* Keep Trans WM disabled for GEN9 */
> + if (IS_GEN9(dev_priv))
> + goto exit;

But the only platforms that call this function are gen9...


> +
> + trans_min = SKL_TRANS_WM_MIN;
> +
> + trans_offset_blocks = (trans_min + SKL_TRANS_WM_AMOUNT) <<
> 16;
> +
> + if (y_tile) {
> + trans_y_tile_min = 2 * y_tile_min;
> + res_blocks = max(trans_y_tile_min, selected_result)
> +
> + trans_offset_blocks;
> + } else {
> + res_blocks = selected_result + trans_offset_blocks;
> + }
> +
> + trans_res_blocks = DIV_ROUND_UP(res_blocks, 1 << 16) + 1;
> +
> + /* WA BUG:1938466 add one block for non y-tile planes */
> + if (!y_tile)
> + trans_res_blocks += 1;
> +exit:
> + *out_blocks = trans_res_blocks;
> + *out_lines = 0;
> +}
> +
> +
>  static int skl_compute_plane_wm(const struct drm_i915_private
> *dev_priv,
>   struct intel_crtc_state *cstate,
>   struct intel_plane_state
> *intel_pstate,
> @@ -3709,6 +3753,9 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>   *out_blocks = res_blocks;
>   *out_lines = res_lines;
>  
> + if (level == 0)
> + skl_compute_plane_trans_wm(dev_priv, pipe_wm,
> intel_pstate,
> + selected_result, y_tile_minimum,
> y_tiled);
>   return 0;
>  }
>  
> @@ -3778,11 +3825,13 @@ skl_compute_linetime_wm(struct
> intel_crtc_state *cstate)
>  }
>  
>  static void skl_compute_transition_wm(struct intel_crtc_state
> *cstate,
> -   struct skl_wm_level *trans_wm
> /* out */)
> +   struct skl_wm_level *trans_wm,
> /* out */
> +   struct skl_ddb_allocation
> *ddb)
>  {
>   struct drm_crtc *crtc = cstate->base.crtc;
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   struct intel_plane *intel_plane;
> + enum pipe pipe = to_intel_crtc(crtc)->pipe;
>  
>   if (!cstate->base.active)
>   return;
> @@ -3790,8 +3839,13 @@ static void skl_compute_transition_wm(struct
> intel_crtc_state *cstate,
>   /* Until we know more, just disable transition WMs */
>   for_each_intel_plane_on_crtc(crtc->dev, intel_crtc,
> intel_plane) {
>   int i = skl_wm_plane_id(intel_plane);
> + uint16_t plane_ddb = skl_ddb_entry_size(
> >plane[pipe][i]);
> + uint16_t trans_res_blocks = trans_wm-
> >plane_res_b[i];
>  
> - trans_wm->plane_en[i] = false;
> + if ((trans_res_blocks > 0) && (trans_res_blocks <=
> plane_ddb))
> + trans_wm->plane_en[i] = true;
> +

Re: [Intel-gfx] [PATCH v3 8/9] drm/i915/bxt: set chicken bit as IPC y-tile WA

[Paulo Zanoni]

Em Sex, 2016-09-09 às 13:31 +0530, Kumar, Mahesh escreveu:
> From: Mahesh Kumar 
> 
> It implements the WA to enable IDLE_WAKEMEM bit of CHICKEN_DCPR_1
> register for Broxton platform. When IPC is enabled & Y-tile is
> enabled in any of the enabled plane, above bit should be set.
> Without this WA system observes random hang.

The previous patch enabled the feature, and now this patch implements a
missing workaround for the feature. This is not the appropriate order,
since it can mean that the previous patch introduced a bug that was
fixed by this patch, and this potentially breaks bisectability. We only
enable the feature once we know all its workarounds are enabled and the
feature will Just Work*. So, normally, my suggestion would be to either
invert the patch order, or just make patches 7 and 8 be a single patch.

But we have another problem, please see
commit 590e8ff04bc0182dce97228e5e352d6413d80456. What's the problem
with the current implementation? Why can't we just keep the bit as 1
forever?


> 
> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h  |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 50
> 
>  3 files changed, 55 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 4737a0e..79b9305 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1632,6 +1632,8 @@ struct skl_wm_values {
>   unsigned dirty_pipes;
>   /* any WaterMark memory workaround Required */
>   enum watermark_memory_wa mem_wa;
> + /* IPC Y-tiled WA related member */
> + u32 y_plane_mask;
>   struct skl_ddb_allocation ddb;
>   uint32_t wm_linetime[I915_MAX_PIPES];
>   uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 75b5b52..210d9b0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5658,6 +5658,9 @@ enum {
>  #define PLANE_NV12_BUF_CFG(pipe, plane)  \
>   _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe),
> _PLANE_NV12_BUF_CFG_2(pipe))
>  
> +#define CHICKEN_DCPR_1 _MMIO(0x46430)
> +#define IDLE_WAKEMEM_MASK  (1 << 13)
> +
>  /* SKL new cursor registers */
>  #define _CUR_BUF_CFG_A   0x7017c
>  #define _CUR_BUF_CFG_B   0x7117c
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5f50f53..ee7f88e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12415,6 +12415,8 @@ 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;
> + struct intel_atomic_state *intel_state =
> + to_intel_atomic_state(plane_state-
> >state);
>   int ret;
>  
>   if (INTEL_GEN(dev) >= 9 && plane->type !=
> DRM_PLANE_TYPE_CURSOR) {
> @@ -12501,6 +12503,15 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>   !needs_scaling(old_plane_state))
>   pipe_config->disable_lp_wm = true;
>  
> + if (fb && (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> + fb->modifier[0] ==
> I915_FORMAT_MOD_Yf_TILED)) {
> + intel_state->wm_results.y_plane_mask |=
> + (1 <<
> drm_plane_index(plane));
> + } else {
> + intel_state->wm_results.y_plane_mask &=
> + ~(1 <<
> drm_plane_index(plane));
> + }
> +
>   return 0;
>  }
>  
> @@ -13963,6 +13974,10 @@ static int intel_atomic_check(struct
> drm_device *dev,
>   if (ret)
>   return ret;
>  
> + /* Copy the Y-tile WA related states */
> + intel_state->wm_results.y_plane_mask =
> + dev_priv-
> >wm.skl_results.y_plane_mask;
> +
>   for_each_crtc_in_state(state, crtc, crtc_state, i) {
>   struct intel_crtc_state *pipe_config =
>   to_intel_crtc_state(crtc_state);
> @@ -14204,6 +14219,32 @@ static void intel_update_crtcs(struct
> drm_atomic_state *state,
>   }
>  }
>  
> +/*
> + * GEN9 IPC WA for Y-tiled
> + */
> +void bxt_set_ipc_wa(struct drm_i915_private *dev_priv, bool enable)
> +{
> + u32 val;
> +
> + if (!IS_BROXTON(dev_priv) || !i915.enable_ipc)
> + return;
> +
> + val = I915_READ(CHICKEN_DCPR_1);
> + /*
> +  * If WA is already enabled or disabled
> +  * no need to re-enable or disable it.
> +  */
> + if ((enable && (val & IDLE_WAKEMEM_MASK)) ||
> + (!enable && !(val & IDLE_WAKEMEM_MASK)))
> 

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/dp: DP audio API changes for MST (rev8)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: DP audio API changes for MST (rev8)
URL   : https://patchwork.freedesktop.org/series/10571/
State : success

== Summary ==

Series 10571v8 drm/i915/dp: DP audio API changes for MST
https://patchwork.freedesktop.org/api/1.0/series/10571/revisions/8/mbox/

Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS   (fi-skl-6770hq)
Test kms_psr_sink_crc:
Subgroup psr_basic:
dmesg-warn -> PASS   (fi-skl-6700hq)

fi-bdw-5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820 total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hqtotal:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2567/

463d07a32d87742a73e1ed352a6d6daa3f29d0c2 drm-intel-nightly: 
2016y-09m-21d-16h-35m-54s UTC integration manifest
a24cc25 drm/i915/dp: DP audio API changes for MST

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 7/9] drm/i915/bxt: Enable IPC support

[Paulo Zanoni]

Hi

Lots of nitpicking in my review. Feel free to disagree with them.


Em Sex, 2016-09-09 às 13:31 +0530, Kumar, Mahesh escreveu:
> From: Mahesh Kumar 
> 
> This patch adds IPC support for platforms. This patch enables IPC
> only for BXT platform as for SKL recommendation is to keep is
> disabled

But don't we want it for KBL too?

Also, can you please elaborate the commit message a little bit more?
What exactly does this feature do? What do we gain with it? Are there
any downsides?

> This patch also adds kernel command-line option to enable/disable
> the IPC if required.

Any reason on why someone would want to do this besides for debugging?
I'm not sure if every single little feature like this one-bit one-
platform feature requires an i915 option, so unless there's a strong
reason, we can just omit the option.

> 

> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/i915_drv.c|  5 +
>  drivers/gpu/drm/i915/i915_params.c |  5 +
>  drivers/gpu/drm/i915/i915_params.h |  1 +
>  drivers/gpu/drm/i915/i915_reg.h|  1 +
>  drivers/gpu/drm/i915/intel_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_pm.c| 21 +
>  6 files changed, 34 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 0a4f18d..22d84e6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1335,6 +1335,11 @@ int i915_driver_load(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  
>   intel_runtime_pm_enable(dev_priv);
>  
> + /*
> +  * Now enable the IPC for supported platforms
> +  */
> + intel_enable_ipc(dev_priv);

The comment is unnecessary: it basically only says what the function
name already says.

Also, I think it makes more sense to move this to intel_init_pm(). As a
bonus, you'll be able to make the function static. Or even just make
the IPC code be part of init_clock_gating() since the whole feature is
just "enable this bit".


> +
>   /* Everything is in place, we can now relax! */
>   DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>    driver.name, driver.major, driver.minor,
> driver.patchlevel,
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 768ad89..cc41b8d 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -62,6 +62,7 @@ struct i915_params i915 __read_mostly = {
>   .inject_load_failure = 0,
>   .enable_dpcd_backlight = false,
>   .enable_gvt = false,
> + .enable_ipc = true,
>  };
>  
>  module_param_named(modeset, i915.modeset, int, 0400);
> @@ -233,3 +234,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
>  module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
>  MODULE_PARM_DESC(enable_gvt,
>   "Enable support for Intel GVT-g graphics virtualization host
> support(default:false)");
> +
> +module_param_named(enable_ipc, i915.enable_ipc, bool, 0400);
> +MODULE_PARM_DESC(enable_ipc,
> + "Enable Isochronous Priority Control
> (default:true)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h
> b/drivers/gpu/drm/i915/i915_params.h
> index 3a0dd78..f99b9b9 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -65,6 +65,7 @@ struct i915_params {
>   bool enable_dp_mst;
>   bool enable_dpcd_backlight;
>   bool enable_gvt;
> + bool enable_ipc;
>  };
>  
>  extern struct i915_params i915 __read_mostly;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index b38445c..75b5b52 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6139,6 +6139,7 @@ enum {
>  #define  DISP_FBC_WM_DIS (1<<15)
>  #define DISP_ARB_CTL2_MMIO(0x45004)
>  #define  DISP_DATA_PARTITION_5_6 (1<<6)
> +#define  DISP_IPC_ENABLE (1<<3)
>  #define DBUF_CTL _MMIO(0x45008)
>  #define  DBUF_POWER_REQUEST  (1<<31)
>  #define  DBUF_POWER_STATE(1<<30)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 66cb46c..56c8ac8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1753,6 +1753,7 @@ void skl_write_plane_wm(struct intel_crtc
> *intel_crtc,
>  uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state
> *pipe_config);
>  bool ilk_disable_lp_wm(struct drm_device *dev);
>  int sanitize_rc6_option(struct drm_i915_private *dev_priv, int
> enable_rc6);
> +void intel_enable_ipc(struct drm_i915_private *dev_priv);
>  static inline int intel_enable_rc6(void)
>  {
>   return i915.enable_rc6;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index d4390e4..8d0037c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4793,6 +4793,27 @@ void 

[Intel-gfx] [CI-1 v5] drm/i915/dp: DP audio API changes for MST

[Dhinakaran Pandiyan]

From: "Pandiyan, Dhinakaran" 

DP MST provides the capability to send multiple video and audio streams
through a single port. This requires the API's between i915 and audio
drivers to distinguish between multiple audio capable displays that can be
connected to a port. Currently only the port identity is shared in the
APIs. This patch adds support for MST with an additional parameter
'int pipe'. The existing parameter 'port' does not change it's meaning.

pipe =
MST : display pipe that the stream originates from
Non-MST : -1

Affected APIs:
struct i915_audio_component_ops
-   int (*sync_audio_rate)(struct device *, int port, int rate);
+   int (*sync_audio_rate)(struct device *, int port, int pipe,
+int rate);

-   int (*get_eld)(struct device *, int port, bool *enabled,
-   unsigned char *buf, int max_bytes);
+   int (*get_eld)(struct device *, int port, int pipe,
+  bool *enabled, unsigned char *buf, int max_bytes);

struct i915_audio_component_audio_ops
-   void (*pin_eld_notify)(void *audio_ptr, int port);
+   void (*pin_eld_notify)(void *audio_ptr, int port, int pipe);

This patch makes dummy changes in the audio drivers (thanks Libin) for
build to succeed. The audio side drivers will send the right 'pipe' values
for MST in patches that will follow.

v2:
Renamed the new API parameter from 'dev_id' to 'pipe'. (Jim, Ville)
Included Asoc driver API compatibility changes from Jeeja.
Added WARN_ON() for invalid pipe in get_saved_encoder(). (Takashi)
Added comment for av_enc_map[] definition. (Takashi)

v3:
Fixed logic error introduced while renaming 'dev_id' as 'pipe' (Ville)
Renamed get_saved_encoder() to get_saved_enc() to reduce line length

v4:
Rebased.
Parameter check for pipe < -1 values in get_saved_enc() (Ville)
Switched to for_each_pipe() in get_saved_enc() (Ville)
Renamed 'pipe' to 'dev_id' in audio side code (Takashi)

v5:
Included a comment for the dev_id arg. (Libin)

Signed-off-by: Dhinakaran Pandiyan 
Reviewed-by: Takashi Iwai 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h|  3 +-
 drivers/gpu/drm/i915/intel_audio.c | 94 ++
 include/drm/i915_component.h   |  6 +--
 include/sound/hda_i915.h   | 11 +++--
 sound/hda/hdac_i915.c  | 18 +---
 sound/pci/hda/patch_hdmi.c |  7 +--
 sound/soc/codecs/hdac_hdmi.c   |  2 +-
 7 files changed, 94 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 270543c..fe14cca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2080,7 +2080,8 @@ struct drm_i915_private {
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
 
-   struct intel_encoder *dig_port_map[I915_MAX_PORTS];
+   /* Used to save the pipe-to-encoder mapping for audio */
+   struct intel_encoder *av_enc_map[I915_MAX_PIPES];
 
/*
 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 40fbdd8..9583f43 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -491,6 +491,7 @@ void intel_audio_codec_enable(struct intel_encoder 
*intel_encoder)
struct drm_i915_private *dev_priv = to_i915(encoder->dev);
struct i915_audio_component *acomp = dev_priv->audio_component;
enum port port = intel_encoder->port;
+   enum pipe pipe = crtc->pipe;
 
connector = drm_select_eld(encoder);
if (!connector)
@@ -515,12 +516,18 @@ void intel_audio_codec_enable(struct intel_encoder 
*intel_encoder)
 
mutex_lock(_priv->av_mutex);
intel_encoder->audio_connector = connector;
+
/* referred in audio callbacks */
-   dev_priv->dig_port_map[port] = intel_encoder;
+   dev_priv->av_enc_map[pipe] = intel_encoder;
mutex_unlock(_priv->av_mutex);
 
+   /* audio drivers expect pipe = -1 to indicate Non-MST cases */
+   if (intel_encoder->type != INTEL_OUTPUT_DP_MST)
+   pipe = -1;
+
if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
-   acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr, 
(int) port);
+   acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+(int) port, (int) pipe);
 }
 
 /**
@@ -536,17 +543,24 @@ void intel_audio_codec_disable(struct intel_encoder 
*intel_encoder)
struct drm_i915_private *dev_priv = to_i915(encoder->dev);
struct i915_audio_component *acomp = dev_priv->audio_component;
enum port port = intel_encoder->port;
+   struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
+   enum 

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915/audio: audio cleanups, 4k fixes

[Patchwork]

== Series Details ==

Series: drm/i915/audio: audio cleanups, 4k fixes
URL   : https://patchwork.freedesktop.org/series/12754/
State : warning

== Summary ==

Series 12754v1 drm/i915/audio: audio cleanups, 4k fixes
https://patchwork.freedesktop.org/api/1.0/series/12754/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup bad-nb-words-3:
pass   -> DMESG-WARN (fi-hsw-4770k)
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS   (fi-skl-6770hq)
Test kms_psr_sink_crc:
Subgroup psr_basic:
dmesg-warn -> PASS   (fi-skl-6700hq)

fi-bdw-5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820 total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k total:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hqtotal:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2566/

463d07a32d87742a73e1ed352a6d6daa3f29d0c2 drm-intel-nightly: 
2016y-09m-21d-16h-35m-54s UTC integration manifest
3f67bf3 drm/i915: set proper N/M in modeset
2884100 drm/i915/audio: rename N value getter to emphasize it's for hdmi
2b6e40d drm/i915/audio: add register macros for audio config N value
5c91e53 drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
44f796c drm/i915/audio: set proper N/MCTS on more platforms
4547db2 drm/i915/audio: split dp and hdmi audio config update
e85597f drm/i915/audio: use the same code for updating audio config
4c06553 drm/i915/audio: port is going to be just fine, simplify checks
1956998 drm/i915/audio: abstract audio config update

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Don't try to handle GuC when GuC is not supported.

[Patchwork]

== Series Details ==

Series: drm/i915: Don't try to handle GuC when GuC is not supported.
URL   : https://patchwork.freedesktop.org/series/12753/
State : success

== Summary ==

Series 12753v1 drm/i915: Don't try to handle GuC when GuC is not supported.
https://patchwork.freedesktop.org/api/1.0/series/12753/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b:
dmesg-warn -> PASS   (fi-skl-6770hq)
Test kms_psr_sink_crc:
Subgroup psr_basic:
dmesg-warn -> PASS   (fi-skl-6700hq)

fi-bdw-5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820 total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hqtotal:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2565/

463d07a32d87742a73e1ed352a6d6daa3f29d0c2 drm-intel-nightly: 
2016y-09m-21d-16h-35m-54s UTC integration manifest
00e8796 drm/i915: Don't try to handle GuC when GuC is not supported.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 0/3] drm/i915: fix some audio support 4K resolution issues

[Jani Nikula]

On Mon, 12 Sep 2016, "Yang, Libin"  wrote:
> Any comments?

Apologies for the way too long delay. I felt it would have been unfair
to ask you to do the changes I thought were necessary after such a
delay, so I sent them as patches... The last one still needs review, but
at least the mundane stuff is out of the way now.

https://patchwork.freedesktop.org/series/12754/

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/9] drm/i915/audio: use the same code for updating audio config

[Jani Nikula]

It gets fragile to duplicate the code for updating HSW_AUD_CFG. The only
change should be that the hdmi pixel clock is also updated in
i915_audio_component_sync_audio_rate(), but it should not be any
different.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 29 +++--
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index aa9bb9318d70..c22574769380 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -631,11 +631,9 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
struct intel_encoder *intel_encoder;
struct intel_crtc *crtc;
-   struct drm_display_mode *mode;
+   struct drm_display_mode *adjusted_mode;
struct i915_audio_component *acomp = dev_priv->audio_component;
enum pipe pipe = INVALID_PIPE;
-   u32 tmp;
-   int n;
int err = 0;
 
/* HSW, BDW, SKL, KBL need this fix */
@@ -666,33 +664,12 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev,
 
DRM_DEBUG_KMS("pipe %c connects port %c\n",
  pipe_name(pipe), port_name(port));
-   mode = >config->base.adjusted_mode;
+   adjusted_mode = >config->base.adjusted_mode;
 
/* port must be valid now, otherwise the pipe will be invalid */
acomp->aud_sample_rate[port] = rate;
 
-   /* 2. check whether to set the N/CTS/M manually or not */
-   if (!audio_rate_need_prog(crtc, mode)) {
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-   goto unlock;
-   }
-
-   n = audio_config_get_n(mode, rate);
-   if (n == 0) {
-   DRM_DEBUG_KMS("Using automatic mode for N value on port %c\n",
- port_name(port));
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-   goto unlock;
-   }
-
-   /* 3. set the N/CTS/M */
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp = audio_config_setup_n_reg(n, tmp);
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+   hsw_audio_config_update(crtc, port, adjusted_mode);
 
  unlock:
mutex_unlock(_priv->av_mutex);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 6/9] drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock

[Jani Nikula]

From: Libin Yang 

HDMI audio should use crtc_clock to get the TMDS clock.

This patch renames mode to adjusted_mode to unify the name.

Reviewed-by: Ville Syrjälä 
Signed-off-by: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 5de24c49b5ed..b73f1f12ba46 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -121,13 +121,14 @@ static u32 audio_config_hdmi_pixel_clock(const struct 
drm_display_mode *adjusted
return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *mode, int rate)
+static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
+ int rate)
 {
int i;
 
for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
if ((rate == aud_ncts[i].sample_rate) &&
-   (mode->clock == aud_ncts[i].clock)) {
+   (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
return aud_ncts[i].n;
}
}
@@ -267,8 +268,8 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
 
-   if (adjusted_mode->clock == TMDS_296M ||
-   adjusted_mode->clock == TMDS_297M) {
+   if (adjusted_mode->crtc_clock == TMDS_296M ||
+   adjusted_mode->crtc_clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/9] drm/i915/audio: split dp and hdmi audio config update

[Jani Nikula]

The code for dp and hdmi are already different, and they're about to
diverge even more. Split them for clarity in future work. No functional
changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 55 +++---
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index c22574769380..edc71e52f732 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -148,18 +148,6 @@ static uint32_t audio_config_setup_n_reg(int n, uint32_t 
val)
return tmp;
 }
 
-/* check whether N/CTS/M need be set manually */
-static bool audio_rate_need_prog(struct intel_crtc *crtc,
-const struct drm_display_mode *mode)
-{
-   if (((mode->clock == TMDS_297M) ||
-(mode->clock == TMDS_296M)) &&
-   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
-   return true;
-   else
-   return false;
-}
-
 static bool intel_eld_uptodate(struct drm_connector *connector,
   i915_reg_t reg_eldv, uint32_t bits_eldv,
   i915_reg_t reg_elda, uint32_t bits_elda,
@@ -245,9 +233,26 @@ static void g4x_audio_codec_enable(struct drm_connector 
*connector,
I915_WRITE(G4X_AUD_CNTL_ST, tmp);
 }
 
-static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
-   enum port port,
-   const struct drm_display_mode 
*adjusted_mode)
+static void
+hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+  const struct drm_display_mode *adjusted_mode)
+{
+   struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+   enum pipe pipe = intel_crtc->pipe;
+   u32 tmp;
+
+   tmp = I915_READ(HSW_AUD_CFG(pipe));
+   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+   tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+   tmp |= AUD_CONFIG_N_VALUE_INDEX;
+
+   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
+static void
+hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+const struct drm_display_mode *adjusted_mode)
 {
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
struct i915_audio_component *acomp = dev_priv->audio_component;
@@ -259,13 +264,11 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-   if (intel_crtc_has_dp_encoder(intel_crtc->config))
-   tmp |= AUD_CONFIG_N_VALUE_INDEX;
-   else
-   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+   if (adjusted_mode->clock == TMDS_296M ||
+   adjusted_mode->clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
@@ -276,6 +279,16 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 }
 
+static void
+hsw_audio_config_update(struct intel_crtc *intel_crtc, enum port port,
+   const struct drm_display_mode *adjusted_mode)
+{
+   if (intel_crtc_has_dp_encoder(intel_crtc->config))
+   hsw_dp_audio_config_update(intel_crtc, port, adjusted_mode);
+   else
+   hsw_hdmi_audio_config_update(intel_crtc, port, adjusted_mode);
+}
+
 static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/9] drm/i915/audio: set proper N/MCTS on more platforms

[Jani Nikula]

From: Libin Yang 

This patch applies setting proper N/M, N/CTS on more platforms.

Reviewed-by: Ville Syrjälä 
Signed-off-by: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index edc71e52f732..5de24c49b5ed 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -649,11 +649,7 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev,
enum pipe pipe = INVALID_PIPE;
int err = 0;
 
-   /* HSW, BDW, SKL, KBL need this fix */
-   if (!IS_SKYLAKE(dev_priv) &&
-   !IS_KABYLAKE(dev_priv) &&
-   !IS_BROADWELL(dev_priv) &&
-   !IS_HASWELL(dev_priv))
+   if (!HAS_DDI(dev_priv))
return 0;
 
i915_audio_component_get_power(kdev);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/9] drm/i915/audio: port is going to be just fine, simplify checks

[Jani Nikula]

If it was wrong, we'd be screwed already.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 1af6812e5955..aa9bb9318d70 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -251,8 +251,9 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
 {
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
struct i915_audio_component *acomp = dev_priv->audio_component;
+   int rate = acomp ? acomp->aud_sample_rate[port] : 0;
enum pipe pipe = intel_crtc->pipe;
-   int n, rate;
+   int n;
u32 tmp;
 
tmp = I915_READ(HSW_AUD_CFG(pipe));
@@ -265,15 +266,6 @@ static void hsw_audio_config_update(struct intel_crtc 
*intel_crtc,
 
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
-   if (!acomp)
-   rate = 0;
-   else if (port >= PORT_A && port <= PORT_E)
-   rate = acomp->aud_sample_rate[port];
-   else {
-   DRM_ERROR("invalid port: %d\n", port);
-   rate = 0;
-   }
-
n = audio_config_get_n(adjusted_mode, rate);
if (n != 0)
tmp = audio_config_setup_n_reg(n, tmp);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 8/9] drm/i915/audio: rename N value getter to emphasize it's for hdmi

[Jani Nikula]

We'll be getting a function and a table for dp parameters soon enough,
so rename the function and table for hdmi. No functional changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index afc325f4dc0d..abfc83363ab0 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -81,7 +81,7 @@ static const struct {
int clock;
int n;
int cts;
-} aud_ncts[] = {
+} hdmi_aud_ncts[] = {
{ 44100, TMDS_296M, 4459, 234375 },
{ 44100, TMDS_297M, 4704, 247500 },
{ 48000, TMDS_296M, 5824, 281250 },
@@ -121,15 +121,15 @@ static u32 audio_config_hdmi_pixel_clock(const struct 
drm_display_mode *adjusted
return hdmi_audio_clock[i].config;
 }
 
-static int audio_config_get_n(const struct drm_display_mode *adjusted_mode,
- int rate)
+static int audio_config_hdmi_get_n(const struct drm_display_mode 
*adjusted_mode,
+  int rate)
 {
int i;
 
-   for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
-   if ((rate == aud_ncts[i].sample_rate) &&
-   (adjusted_mode->crtc_clock == aud_ncts[i].clock)) {
-   return aud_ncts[i].n;
+   for (i = 0; i < ARRAY_SIZE(hdmi_aud_ncts); i++) {
+   if (rate == hdmi_aud_ncts[i].sample_rate &&
+   adjusted_mode->crtc_clock == hdmi_aud_ncts[i].clock) {
+   return hdmi_aud_ncts[i].n;
}
}
return 0;
@@ -256,7 +256,7 @@ hsw_hdmi_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
 
if (adjusted_mode->crtc_clock == TMDS_296M ||
adjusted_mode->crtc_clock == TMDS_297M) {
-   n = audio_config_get_n(adjusted_mode, rate);
+   n = audio_config_hdmi_get_n(adjusted_mode, rate);
if (n != 0) {
tmp &= ~AUD_CONFIG_N_MASK;
tmp |= AUD_CONFIG_N(n);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 9/9] drm/i915: set proper N/M in modeset

[Jani Nikula]

From: Libin Yang 

When modeset occurs and the LS_CLK is set to some
special values in DP mode, the N/M need to be set
manually if audio is playing. Otherwise the first
several seconds may be silent in audio playback.

The relationship of Maud and Naud is expressed in
the following equation:
Maud/Naud = 512 * fs / f_LS_Clk

Please refer VESA DisplayPort Standard spec for details.

Signed-off-by: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_reg.h|   7 +++
 drivers/gpu/drm/i915/intel_audio.c | 100 -
 2 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc04ce68f8b6..2cde9513 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7358,6 +7358,13 @@ enum {
 #define _HSW_AUD_MISC_CTRL_B   0x65110
 #define HSW_AUD_MISC_CTRL(pipe)_MMIO_PIPE(pipe, 
_HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
 
+#define _HSW_AUD_M_CTS_ENABLE_A0x65028
+#define _HSW_AUD_M_CTS_ENABLE_B0x65128
+#define HSW_AUD_M_CTS_ENABLE(pipe) _MMIO_PIPE(pipe, 
_HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
+#define   AUD_M_CTS_M_VALUE_INDEX  (1 << 21)
+#define   AUD_M_CTS_M_PROG_ENABLE  (1 << 20)
+#define   AUD_CONFIG_M_MASK0xf
+
 #define _HSW_AUD_DIP_ELD_CTRL_ST_A 0x650b4
 #define _HSW_AUD_DIP_ELD_CTRL_ST_B 0x651b4
 #define HSW_AUD_DIP_ELD_CTRL(pipe) _MMIO_PIPE(pipe, 
_HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index abfc83363ab0..e81dd86fe523 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -57,6 +57,70 @@
  * struct _audio_component_audio_ops @audio_ops is called from i915 
driver.
  */
 
+/* DP N/M table */
+#define LC_540M 54
+#define LC_270M 27
+#define LC_162M 162000
+
+struct dp_aud_n_m {
+   int sample_rate;
+   int clock;
+   u16 n;
+   u16 m;
+};
+
+static const struct dp_aud_n_m dp_aud_n_m[] = {
+   { 192000, LC_540M, 5625, 1024 },
+   { 176400, LC_540M, 9375, 1568 },
+   { 96000, LC_540M, 5625, 512 },
+   { 88200, LC_540M, 9375, 784 },
+   { 48000, LC_540M, 5625, 256 },
+   { 44100, LC_540M, 9375, 392 },
+   { 32000, LC_540M, 16875, 512 },
+   { 192000, LC_270M, 5625, 2048 },
+   { 176400, LC_270M, 9375, 3136 },
+   { 96000, LC_270M, 5625, 1024 },
+   { 88200, LC_270M, 9375, 1568 },
+   { 48000, LC_270M, 5625, 512 },
+   { 44100, LC_270M, 9375, 784 },
+   { 32000, LC_270M, 16875, 1024 },
+   { 192000, LC_162M, 3375, 2048 },
+   { 176400, LC_162M, 5625, 3136 },
+   { 96000, LC_162M, 3375, 1024 },
+   { 88200, LC_162M, 5625, 1568 },
+   { 48000, LC_162M, 3375, 512 },
+   { 44100, LC_162M, 5625, 784 },
+   { 32000, LC_162M, 10125, 1024 },
+};
+
+static const struct dp_aud_n_m *
+audio_config_dp_get_n_m(struct intel_crtc *intel_crtc, int rate)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(dp_aud_n_m); i++) {
+   if (rate == dp_aud_n_m[i].sample_rate &&
+   intel_crtc->config->port_clock == dp_aud_n_m[i].clock)
+   return _aud_n_m[i];
+   }
+
+   return NULL;
+}
+
+static int audio_config_dp_get_m(struct intel_crtc *intel_crtc, int rate)
+{
+   const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate);
+
+   return nm ? nm->m : 0;
+}
+
+static int audio_config_dp_get_n(struct intel_crtc *intel_crtc, int rate)
+{
+   const struct dp_aud_n_m *nm = audio_config_dp_get_n_m(intel_crtc, rate);
+
+   return nm ? nm->n : 0;
+}
+
 static const struct {
int clock;
u32 config;
@@ -225,8 +289,10 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
   const struct drm_display_mode *adjusted_mode)
 {
struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+   struct i915_audio_component *acomp = dev_priv->audio_component;
+   int rate = acomp ? acomp->aud_sample_rate[port] : 0;
enum pipe pipe = intel_crtc->pipe;
-   u32 tmp;
+   u32 tmp, n, m;
 
tmp = I915_READ(HSW_AUD_CFG(pipe));
tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
@@ -234,7 +300,30 @@ hsw_dp_audio_config_update(struct intel_crtc *intel_crtc, 
enum port port,
tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
tmp |= AUD_CONFIG_N_VALUE_INDEX;
 
+   if (intel_crtc->config->port_clock == LC_540M ||
+   intel_crtc->config->port_clock == LC_270M ||
+   intel_crtc->config->port_clock == LC_162M) {
+   n = audio_config_dp_get_n(intel_crtc, rate);
+   if (n != 0) {
+   tmp &= ~AUD_CONFIG_N_MASK;
+   tmp |= 

[Intel-gfx] [PATCH 0/9] drm/i915/audio: audio cleanups, 4k fixes

[Jani Nikula]

I started to review Libin's series at [1], but decided that
intel_audio.c is badly in need of some cleanup before moving forward
with the patches. I think the prep work makes Libin's patches cleaner
too. I ended up rebasing the patches myself, but it was a rather quick
job, with no testing, so please do double check.

BR,
Jani.


[1] https://patchwork.freedesktop.org/series/11252/


Jani Nikula (6):
  drm/i915/audio: abstract audio config update
  drm/i915/audio: port is going to be just fine, simplify checks
  drm/i915/audio: use the same code for updating audio config
  drm/i915/audio: split dp and hdmi audio config update
  drm/i915/audio: add register macros for audio config N value
  drm/i915/audio: rename N value getter to emphasize it's for hdmi

Libin Yang (3):
  drm/i915/audio: set proper N/MCTS on more platforms
  drm/i915/audio: HDMI audio gets the TMDS clock by crtc_clock
  drm/i915: set proper N/M in modeset

 drivers/gpu/drm/i915/i915_reg.h|  11 ++
 drivers/gpu/drm/i915/intel_audio.c | 260 -
 2 files changed, 179 insertions(+), 92 deletions(-)

-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 7/9] drm/i915/audio: add register macros for audio config N value

[Jani Nikula]

Have generic macros in line with the rest of the register bit definition
macros instead of a dedicated function in intel_audio.c, and use them.
No functional changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_reg.h|  4 
 drivers/gpu/drm/i915/intel_audio.c | 23 ++-
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8d44cee710f0..dc04ce68f8b6 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7331,6 +7331,10 @@ enum {
 #define   AUD_CONFIG_UPPER_N_MASK  (0xff << 20)
 #define   AUD_CONFIG_LOWER_N_SHIFT 4
 #define   AUD_CONFIG_LOWER_N_MASK  (0xfff << 4)
+#define   AUD_CONFIG_N_MASK(AUD_CONFIG_UPPER_N_MASK | 
AUD_CONFIG_LOWER_N_MASK)
+#define   AUD_CONFIG_N(n) \
+   (n) >> 12) & 0xff) << AUD_CONFIG_UPPER_N_SHIFT) |   \
+(((n) & 0xfff) << AUD_CONFIG_LOWER_N_SHIFT))
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_SHIFT16
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK (0xf << 16)
 #define   AUD_CONFIG_PIXEL_CLOCK_HDMI_25175(0 << 16)
diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index b73f1f12ba46..afc325f4dc0d 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -135,20 +135,6 @@ static int audio_config_get_n(const struct 
drm_display_mode *adjusted_mode,
return 0;
 }
 
-static uint32_t audio_config_setup_n_reg(int n, uint32_t val)
-{
-   int n_low, n_up;
-   uint32_t tmp = val;
-
-   n_low = n & 0xfff;
-   n_up = (n >> 12) & 0xff;
-   tmp &= ~(AUD_CONFIG_UPPER_N_MASK | AUD_CONFIG_LOWER_N_MASK);
-   tmp |= ((n_up << AUD_CONFIG_UPPER_N_SHIFT) |
-   (n_low << AUD_CONFIG_LOWER_N_SHIFT) |
-   AUD_CONFIG_N_PROG_ENABLE);
-   return tmp;
-}
-
 static bool intel_eld_uptodate(struct drm_connector *connector,
   i915_reg_t reg_eldv, uint32_t bits_eldv,
   i915_reg_t reg_elda, uint32_t bits_elda,
@@ -271,10 +257,13 @@ hsw_hdmi_audio_config_update(struct intel_crtc 
*intel_crtc, enum port port,
if (adjusted_mode->crtc_clock == TMDS_296M ||
adjusted_mode->crtc_clock == TMDS_297M) {
n = audio_config_get_n(adjusted_mode, rate);
-   if (n != 0)
-   tmp = audio_config_setup_n_reg(n, tmp);
-   else
+   if (n != 0) {
+   tmp &= ~AUD_CONFIG_N_MASK;
+   tmp |= AUD_CONFIG_N(n);
+   tmp |= AUD_CONFIG_N_PROG_ENABLE;
+   } else {
DRM_DEBUG_KMS("no suitable N value is found\n");
+   }
}
 
I915_WRITE(HSW_AUD_CFG(pipe), tmp);
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/9] drm/i915/audio: abstract audio config update

[Jani Nikula]

Prepare for using the same code for updating HSW_AUD_CFG register. No
functional changes.

Cc: Libin Yang 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/intel_audio.c | 68 ++
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c 
b/drivers/gpu/drm/i915/intel_audio.c
index 40fbdd851520..1af6812e5955 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -245,6 +245,45 @@ static void g4x_audio_codec_enable(struct drm_connector 
*connector,
I915_WRITE(G4X_AUD_CNTL_ST, tmp);
 }
 
+static void hsw_audio_config_update(struct intel_crtc *intel_crtc,
+   enum port port,
+   const struct drm_display_mode 
*adjusted_mode)
+{
+   struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+   struct i915_audio_component *acomp = dev_priv->audio_component;
+   enum pipe pipe = intel_crtc->pipe;
+   int n, rate;
+   u32 tmp;
+
+   tmp = I915_READ(HSW_AUD_CFG(pipe));
+   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
+   tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
+   if (intel_crtc_has_dp_encoder(intel_crtc->config))
+   tmp |= AUD_CONFIG_N_VALUE_INDEX;
+   else
+   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
+
+   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
+   if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
+   if (!acomp)
+   rate = 0;
+   else if (port >= PORT_A && port <= PORT_E)
+   rate = acomp->aud_sample_rate[port];
+   else {
+   DRM_ERROR("invalid port: %d\n", port);
+   rate = 0;
+   }
+
+   n = audio_config_get_n(adjusted_mode, rate);
+   if (n != 0)
+   tmp = audio_config_setup_n_reg(n, tmp);
+   else
+   DRM_DEBUG_KMS("no suitable N value is found\n");
+   }
+
+   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+}
+
 static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 {
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -283,11 +322,9 @@ static void hsw_audio_codec_enable(struct drm_connector 
*connector,
struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
enum pipe pipe = intel_crtc->pipe;
enum port port = intel_encoder->port;
-   struct i915_audio_component *acomp = dev_priv->audio_component;
const uint8_t *eld = connector->eld;
uint32_t tmp;
int len, i;
-   int n, rate;
 
DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
  pipe_name(pipe), drm_eld_size(eld));
@@ -323,32 +360,7 @@ static void hsw_audio_codec_enable(struct drm_connector 
*connector,
I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
 
/* Enable timestamps */
-   tmp = I915_READ(HSW_AUD_CFG(pipe));
-   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
-   tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-   if (intel_crtc_has_dp_encoder(intel_crtc->config))
-   tmp |= AUD_CONFIG_N_VALUE_INDEX;
-   else
-   tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
-
-   tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
-   if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
-   if (!acomp)
-   rate = 0;
-   else if (port >= PORT_A && port <= PORT_E)
-   rate = acomp->aud_sample_rate[port];
-   else {
-   DRM_ERROR("invalid port: %d\n", port);
-   rate = 0;
-   }
-   n = audio_config_get_n(adjusted_mode, rate);
-   if (n != 0)
-   tmp = audio_config_setup_n_reg(n, tmp);
-   else
-   DRM_DEBUG_KMS("no suitable N value is found\n");
-   }
-
-   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
+   hsw_audio_config_update(intel_crtc, port, adjusted_mode);
 
mutex_unlock(_priv->av_mutex);
 }
-- 
2.1.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 6/9] drm/i915/skl+: change WM calc to fixed point 16.16

[Paulo Zanoni]

Hi

Em Sex, 2016-09-09 às 13:31 +0530, Kumar, Mahesh escreveu:
> From: Mahesh Kumar 

First of all, good catch with this patch!

> 
> This patch changes Watermak calculation to fixed point calculation.
> Problem with current calculation is during plane_blocks_per_line
> calculation we divide intermediate blocks with min_scanlines and
> takes floor of the result because of integer operation.
> hence we end-up assigning less blocks than required. Which leads to
> flickers.

This problem is that the patch doesn't only do this. It also tries to
make the method1 result be a fixed point 16.16, and it also does a
completely unrelated change with the addition of the y_tiled variable.
I'd really like of the 3 changes were 3 different patches.


> 
> Signed-off-by: Mahesh Kumar 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 0ec328b..d4390e4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3530,13 +3530,15 @@ static uint32_t skl_pipe_pixel_rate(const
> struct intel_crtc_state *config)
>  */
>  static uint32_t skl_wm_method1(uint32_t pixel_rate, uint8_t cpp,
> uint32_t latency)
>  {
> - uint32_t wm_intermediate_val, ret;
> + uint64_t wm_intermediate_val;
> + uint32_t ret;
>  
>   if (latency == 0)
>   return UINT_MAX;
>  
> - wm_intermediate_val = latency * pixel_rate * cpp / 512;
> - ret = DIV_ROUND_UP(wm_intermediate_val, 1000);
> + wm_intermediate_val = latency * pixel_rate * cpp;
> + wm_intermediate_val <<= 16;
> + ret = DIV_ROUND_UP_ULL(wm_intermediate_val, 1000 * 512);
>  
>   return ret;
>  }

So shouldn't we fix the callers of skl_wm_method1 to take into
consideration that the value returned is now a fixed point 16.16?
Sounds like we have a bug here.

Also, having method1 be 16.16 and method2 be a normal integer adds a
lot to the confusion.


> @@ -3605,6 +3607,7 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>   uint16_t *out_blocks = >plane_res_b[id];
>   uint8_t *out_lines = >plane_res_l[id];
>   enum watermark_memory_wa mem_wa;
> + bool y_tiled = false;

Please either discard these y_tiled chunks or move them to a separate
patch. And since we have the y_tiled variable, maybe we'd also like to
have an x_tiled variable for consistency between the checks?


>  
>   if (latency == 0 || !cstate->base.active || !intel_pstate-
> >base.visible)
>   return 0;
> @@ -3652,14 +3655,18 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>   plane_bytes_per_line = width * cpp;
>   if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>   fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> + y_tiled = true;
>   plane_blocks_per_line =
>     DIV_ROUND_UP(plane_bytes_per_line *
> y_min_scanlines, 512);
> - plane_blocks_per_line /= y_min_scanlines;
> + plane_blocks_per_line = (plane_blocks_per_line <<
> 16) /
> + y_mi
> n_scanlines;
>   } else if (fb->modifier[0] == DRM_FORMAT_MOD_NONE) {
>   plane_blocks_per_line =
> DIV_ROUND_UP(plane_bytes_per_line, 512)
>   + 1;
> + plane_blocks_per_line <<= 16;
>   } else {
>   plane_blocks_per_line =
> DIV_ROUND_UP(plane_bytes_per_line, 512);
> + plane_blocks_per_line <<= 16;
>   }

This is probably a rebase problem, but not all places where
plane_blocks_per_line are used were fixed to take into consideration
the new 16.16 format.


Now, what I've been thinking is that we completely fail at type-safety
when we mix these normal integers with the 16.16 integers. Ideally, we
should find a way to make the compiler complain about this to us, since
it's too easy to make such mistakes. Can't we try to make the compiler
help us, such as by defining something like

typedef struct {
uint32_t val;
} uint_fixed_16_16_t;

and then making the appropriate functions/macros for maniuplating them
and mixing them with integers? This would help catching problems such
as the one we have here. Also, other pieces of i915.ko and drm.ko could
benefit from this.

>  
>   method1 = skl_wm_method1(plane_pixel_rate, cpp, latency);
> @@ -3670,8 +3677,7 @@ static int skl_compute_plane_wm(const struct
> drm_i915_private *dev_priv,
>  
>   y_tile_minimum = plane_blocks_per_line * y_min_scanlines;
>  
> - if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> - fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> + if (y_tiled) {
>   selected_result = max(method2, y_tile_minimum);
>   } else {
>   uint32_t linetime_us = 0;
> @@ -3688,12 

[Intel-gfx] [PATCH] drm/i915: Don't try to handle GuC when GuC is not supported.

[Rodrigo Vivi]

Avoid any kind of GuC handling if GuC is not supported
on a giving platform.

Besides being useless handling, our driver needs
to be smarter than the user trying to use an invalid paramenter.

Cc: Jani Nikula 
Cc: Anusha Srivatsa 
Cc: Christophe Prigent 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97573
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 6fd39ef..da0f5ed 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -720,6 +720,13 @@ void intel_guc_init(struct drm_device *dev)
struct intel_guc_fw *guc_fw = _priv->guc.guc_fw;
const char *fw_path;
 
+   if (!HAS_GUC(dev)) {
+   i915.enable_guc_loading = 0;
+   i915.enable_guc_submission = 0;
+   fw_path = NULL;
+   return;
+   }
+
/* A negative value means "use platform default" */
if (i915.enable_guc_loading < 0)
i915.enable_guc_loading = HAS_GUC_UCODE(dev);
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/3] drm/i915: Restore current RPS state after reset

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/3] drm/i915: Restore current RPS state after 
reset
URL   : https://patchwork.freedesktop.org/series/12741/
State : failure

== Summary ==

Series 12741v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/12741/revisions/1/mbox/

Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass   -> DMESG-WARN (fi-skl-6770hq)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass   -> INCOMPLETE (fi-hsw-4770k)

fi-bdw-5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-byt-n2820 total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770k total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hqtotal:244  pass:227  dwarn:2   dfail:0   fail:1   skip:14 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2564/

395eb4879cae6cc107c9f673008ae2a77bd43c66 drm-intel-nightly: 
2016y-09m-21d-13h-42m-23s UTC integration manifest
d4bba6e drm/i915/execlists: Reset RING registers upon resume
120a221 drm/i915: Only shrink the unbound objects during freeze
d92d44b drm/i915: Restore current RPS state after reset

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

[Maiti, Nabendu Bikash]

Hi,


On 9/20/2016 1:55 PM, Ville Syrjälä wrote:

On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:

Following patch series add pipe scaler functionality in atomic path.The pipe
scaler can be changed dynamically without modeset.Apart from default panel
fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
mode is added as there is no such restriction on GEn9 pipe scaler.


Some quick observations:
- missing any interaction with drm core, so all generic fb size checks
  and whatnot will not work out, I think
Pipe scaler is not dependent on fp I think. We have fb size checks are 
done in plane check.



- the way it's done goes against what I've suggested in the past. My
  idea has been to add a "fixed mode" property to connectors instead,
  which I think would minimize the impact on the core, and it would
  follow closely the way eDP/LVDS/DSI already works today.
yes using fixed mode we can do also but I wanted to be part of crtc 
property instead of connector property. As fixed mode is basically 
intended for fixed mode panels.But we may use pipe scaler for fixed mode 
and dynamic mode panels.



- There's no need to restrict the feature to gen9+ only. It should work
  out just fine for at least all pch platforms. gmch platforms would be
  more challenging
This code I designed to use gen9+, and properties like crtc destination 
size and offsets also exposed.There is no restrictions on modes (eg. 
pillerbox/letterbox) and down scaling ratios as previous platforms. 
Currently scaling mode is part of connector property and implemented as 
legacy property. I created new scaling mode as atomic property. I think 
gen9+ onward platforms may have proper atomic pipe scaling properties 
and user space may use it fully dynamically without modeset.



- the pfiter_recalculate thing looks pretty wrong atomic wise
Sorry, I couldn't get it. Are you referring pipe scaler registers are 
not written together with other registers?  pfiter_calculate only 
calculate and stores the data for later commit. Please provide more 
details on it.






Nabendu Maiti (7):
  drm/i915: Add pipe scaler pipe source drm property
  drm/i915: Add pipe_src size property calculations in atomic path.
  drm/i915: Panel fitting calculation for GEN9
  drm/i915: Adding atomic fitting mode property for GEN9
  drm/i915: Add pipe scaler co-ordinate and size property for Gen9
  drm/i915: Update pipe-scaler according to destination size
  drm/i915: Pipescaler destination size limit check on Gen9

 drivers/gpu/drm/drm_atomic.c |  35 ++
 drivers/gpu/drm/drm_crtc.c   |  56 +++
 drivers/gpu/drm/i915/intel_display.c | 128 +--
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 drivers/gpu/drm/i915/intel_panel.c   |  34 +-
 include/drm/drm_crtc.h   |  14 
 include/uapi/drm/drm_mode.h  |   1 +
 7 files changed, 263 insertions(+), 8 deletions(-)

--
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




--
Regards,
Nabendu
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 1/9] drm/i915/skl: pass pipe_wm in skl_compute_(wm_level/plane_wm) functions

[Paulo Zanoni]

Em Qua, 2016-09-21 às 19:18 +0530, Mahesh Kumar escreveu:
> there are two main reason I want to pass complete pipe_wm to compute 
> function
> 
> 1. for transition WM calculation, else we need to pass 
> trans_wm.plane_res_b &  trans_wm.plane_res_l also as a parameter,
> 
>  that will increase the parameter-list. And trans_wm will be
> used 
> (calculation will happen) only during level-0. All other levels (1-
> 7) 
> these will be extra unused variables.
> 
>  If we want to calculate transition WM separately (after plane
> WM 
> calculation) there we will have to duplicate some of the code for WM 
> level-0 calculation.
> 
> 2. This will be used in Render compression WA for (Gen9 & CNL) where
> we 
> have to make sure WM level-(1-7) result blocks/lines are >= WM level-
> 0 
> result blocks/lines.

Well, I tried to check against the series and I really thought they
were not necessary.

Considering there's also some discussion whether we want transition WMs
or not (i.e., the HW guys say we don't want), can you please at least
move this patch to be right before the point where it is needed? This
way we'll be able to discuss & merge the others before.

Thanks,
Paulo

> 
> 
> agree, out_ variable should not be local variables.
> If you think above points can be addressed, let me know, will update 
> patches accordingly.
> 
> thanks,
> -Mahesh
> 
> On Tuesday 20 September 2016 05:47 PM, Paulo Zanoni wrote:
> > 
> > Em Sex, 2016-09-09 às 13:30 +0530, Kumar, Mahesh escreveu:
> > > 
> > > From: Mahesh Kumar 
> > > 
> > > This patch make use of plane_wm variable directly instead of
> > > passing
> > > skl_plane_wm struct. this way reduces number of argument
> > > requirement
> > > in watermark calculation functions.
> > > 
> > > It also gives more freedom of decision making to implement Bspec
> > > WM
> > > workarounds.
> > This is just my personal opinion, but I don't think this patch is
> > an
> > improvement to the codebase. The way things are currently
> > organized,
> > there's no risk that we may end up reading some variable that's
> > still
> > not set/computed, so there's less risk that some later patch may
> > end up
> > using information it shouldn't. Also, by having these explicit out
> > parameters, we clearly highlight what's the goal of the function:
> > output those 3 values, instead of writing I-don't-know-what to a
> > huge
> > struct.
> > 
> > Besides, the patch even keeps the out_ variables as local variables
> > instead of parameters, which makes things even more confusing IMHO,
> > since in_ and out_ variables are usually function parameters.
> > 
> > I also see that this patch is not necessary for the series. We
> > kinda
> > use the new pipe_wm variable at patch 9, but we could just go with
> > the
> > variables we have.
> > 
> > So, unless some new arguments are given, I'd suggest to just drop
> > this
> > patch.
> > 
> > > 
> > > Signed-off-by: Mahesh Kumar 
> > > ---
> > >   drivers/gpu/drm/i915/intel_pm.c | 29 +++---
> > > ---
> > >   1 file changed, 15 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 86c6d66..3fdec4d 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -3542,9 +3542,7 @@ static int skl_compute_plane_wm(const
> > > struct
> > > drm_i915_private *dev_priv,
> > >   struct intel_plane_state
> > > *intel_pstate,
> > >   uint16_t ddb_allocation,
> > >   int level,
> > > - uint16_t *out_blocks, /* out */
> > > - uint8_t *out_lines, /* out */
> > > - bool *enabled /* out */)
> > > + struct skl_pipe_wm *pipe_wm)
> > >   {
> > >   struct drm_plane_state *pstate = _pstate->base;
> > >   struct drm_framebuffer *fb = pstate->fb;
> > > @@ -3557,6 +3555,11 @@ static int skl_compute_plane_wm(const
> > > struct
> > > drm_i915_private *dev_priv,
> > >   uint32_t width = 0, height = 0;
> > >   uint32_t plane_pixel_rate;
> > >   uint32_t y_tile_minimum, y_min_scanlines;
> > > + int id = skl_wm_plane_id(to_intel_plane(pstate->plane));
> > > + struct skl_wm_level *result = _wm->wm[level];
> > > + uint16_t *out_blocks = >plane_res_b[id];
> > > + uint8_t *out_lines = >plane_res_l[id];
> > > + bool *enabled = >plane_en[id];
> > >   
> > >   if (latency == 0 || !cstate->base.active ||
> > > !intel_pstate-
> > > > 
> > > > base.visible) {
> > >   *enabled = false;
> > > @@ -3673,7 +3676,7 @@ skl_compute_wm_level(const struct
> > > drm_i915_private *dev_priv,
> > >    struct skl_ddb_allocation *ddb,
> > >    struct intel_crtc_state *cstate,
> > >    int level,
> > > -

[Intel-gfx] [CI 1/3] drm/i915: Restore current RPS state after reset

[Chris Wilson]

Following commit 821ed7df6e2a ("drm/i915: Update reset path to fix
incomplete requests") we no longer mark the context as lost on reset as
we keep the requests (and contexts) alive. However, RPS remains reset
and we need to restore the current state to match the in-flight
requests.

Signed-off-by: Chris Wilson 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97824
Fixes: 821ed7df6e2a ("drm/i915: Update reset path to fix incomplete requests")
Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Tvrtko Ursulin 
Cc: Arun Siluvery 
Reviewed-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.c | 9 -
 drivers/gpu/drm/i915/i915_gem.c | 7 +++
 drivers/gpu/drm/i915/i915_irq.c | 6 ++
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7f4e8adec8a8..8ae5853ea3c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1786,15 +1786,6 @@ void i915_reset(struct drm_i915_private *dev_priv)
goto error;
}
 
-   /*
-* rps/rc6 re-init is necessary to restore state lost after the
-* reset and the re-install of gt irqs. Skip for ironlake per
-* previous concerns that it doesn't respond well to some forms
-* of re-init after reset.
-*/
-   intel_sanitize_gt_powersave(dev_priv);
-   intel_autoenable_gt_powersave(dev_priv);
-
 wakeup:
wake_up_bit(>flags, I915_RESET_IN_PROGRESS);
return;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c8bd02277b7d..aeb46658ab3c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2628,6 +2628,13 @@ void i915_gem_reset(struct drm_i915_private *dev_priv)
i915_gem_reset_engine(engine);
 
i915_gem_restore_fences(_priv->drm);
+
+   if (dev_priv->gt.awake) {
+   intel_sanitize_gt_powersave(dev_priv);
+   intel_enable_gt_powersave(dev_priv);
+   if (INTEL_GEN(dev_priv) >= 6)
+   gen6_rps_busy(dev_priv);
+   }
 }
 
 static void nop_submit_request(struct drm_i915_gem_request *request)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a5c02f6ea6a0..f8c0beaadf30 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -350,6 +350,9 @@ void gen6_reset_rps_interrupts(struct drm_i915_private 
*dev_priv)
 
 void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv)
 {
+   if (READ_ONCE(dev_priv->rps.interrupts_enabled))
+   return;
+
spin_lock_irq(_priv->irq_lock);
WARN_ON_ONCE(dev_priv->rps.pm_iir);
WARN_ON_ONCE(I915_READ(gen6_pm_iir(dev_priv)) & 
dev_priv->pm_rps_events);
@@ -368,6 +371,9 @@ u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private 
*dev_priv, u32 mask)
 
 void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
 {
+   if (!READ_ONCE(dev_priv->rps.interrupts_enabled))
+   return;
+
spin_lock_irq(_priv->irq_lock);
dev_priv->rps.interrupts_enabled = false;
 
-- 
2.9.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [CI 3/3] drm/i915/execlists: Reset RING registers upon resume

[Chris Wilson]

There is a disparity in the context image saved to disk and our own
bookkeeping - that is we presume the RING_HEAD and RING_TAIL match our
stored ce->ring->tail value. However, as we emit WA_TAIL_DWORDS into the
ring but may not tell the GPU about them, the GPU may be lagging behind
our bookkeeping. Upon hibernation we do not save stolen pages, presuming
that their contents are volatile. This means that although we start
writing into the ring at tail, the GPU starts executing from its HEAD
and there may be some garbage in between and so the GPU promptly hangs
upon resume.

Testcase: igt/gem_exec_suspend/basic-S4
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96526
Signed-off-by: Chris Wilson 
Reviewed-by: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/intel_lrc.c | 50 +---
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 251143361f31..39417b77bff2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2129,30 +2129,42 @@ error_deref_obj:
 
 void intel_lr_context_resume(struct drm_i915_private *dev_priv)
 {
-   struct i915_gem_context *ctx = dev_priv->kernel_context;
struct intel_engine_cs *engine;
+   struct i915_gem_context *ctx;
+
+   /* Because we emit WA_TAIL_DWORDS there may be a disparity
+* between our bookkeeping in ce->ring->head and ce->ring->tail and
+* that stored in context. As we only write new commands from
+* ce->ring->tail onwards, everything before that is junk. If the GPU
+* starts reading from its RING_HEAD from the context, it may try to
+* execute that junk and die.
+*
+* So to avoid that we reset the context images upon resume. For
+* simplicity, we just zero everything out.
+*/
+   list_for_each_entry(ctx, _priv->context_list, link) {
+   for_each_engine(engine, dev_priv) {
+   struct intel_context *ce = >engine[engine->id];
+   u32 *reg;
 
-   for_each_engine(engine, dev_priv) {
-   struct intel_context *ce = >engine[engine->id];
-   void *vaddr;
-   uint32_t *reg_state;
-
-   if (!ce->state)
-   continue;
-
-   vaddr = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
-   if (WARN_ON(IS_ERR(vaddr)))
-   continue;
+   if (!ce->state)
+   continue;
 
-   reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE;
+   reg = i915_gem_object_pin_map(ce->state->obj,
+ I915_MAP_WB);
+   if (WARN_ON(IS_ERR(reg)))
+   continue;
 
-   reg_state[CTX_RING_HEAD+1] = 0;
-   reg_state[CTX_RING_TAIL+1] = 0;
+   reg += LRC_STATE_PN * PAGE_SIZE / sizeof(*reg);
+   reg[CTX_RING_HEAD+1] = 0;
+   reg[CTX_RING_TAIL+1] = 0;
 
-   ce->state->obj->dirty = true;
-   i915_gem_object_unpin_map(ce->state->obj);
+   ce->state->obj->dirty = true;
+   i915_gem_object_unpin_map(ce->state->obj);
 
-   ce->ring->head = 0;
-   ce->ring->tail = 0;
+   ce->ring->head = ce->ring->tail = 0;
+   ce->ring->last_retired_head = -1;
+   intel_ring_update_space(ce->ring);
+   }
}
 }
-- 
2.9.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [CI 2/3] drm/i915: Only shrink the unbound objects during freeze

[Chris Wilson]

At the point of creating the hibernation image, the runtime power manage
core is disabled - and using the rpm functions triggers a warn.
i915_gem_shrink_all() tries to unbind objects, which requires device
access and so tries to how an rpm reference triggering a warning:

[   44.235420] [ cut here ]
[   44.235424] WARNING: CPU: 2 PID: 2199 at 
drivers/gpu/drm/i915/intel_runtime_pm.c:2688 
intel_runtime_pm_get_if_in_use+0xe6/0xf0
[   44.235426] WARN_ON_ONCE(ret < 0)
[   44.235445] Modules linked in: ctr ccm arc4 rt2800usb rt2x00usb rt2800lib 
rt2x00lib crc_ccitt mac80211 cmac cfg80211 btusb rfcomm bnep btrtl btbcm 
btintel bluetooth dcdbas x86_pkg_temp_thermal intel_powerclamp coretemp 
snd_hda_codec_realtek crct10dif_pclmul crc32_pclmul ghash_clmulni_intel 
snd_hda_codec_generic aesni_intel snd_hda_codec_hdmi aes_x86_64 lrw gf128mul 
snd_hda_intel glue_helper ablk_helper cryptd snd_hda_codec hid_multitouch 
joydev snd_hda_core binfmt_misc i2c_hid serio_raw snd_pcm acpi_pad snd_timer 
snd i2c_designware_platform 8250_dw nls_iso8859_1 i2c_designware_core lpc_ich 
mfd_core soundcore usbhid hid psmouse ahci libahci
[   44.235447] CPU: 2 PID: 2199 Comm: kworker/u8:8 Not tainted 4.8.0-rc5+ #130
[   44.235447] Hardware name: Dell Inc. XPS 13 9343/0310JH, BIOS A07 11/11/2015
[   44.235450] Workqueue: events_unbound async_run_entry_fn
[   44.235453]   8801b2f7fb98 81306c2f 
8801b2f7fbe8
[   44.235454]   8801b2f7fbd8 81056c01 
0a801f50ecc0
[   44.235456]  88020ce5 88020ce59b60 81a60b5c 
81414840
[   44.235456] Call Trace:
[   44.235459]  [] dump_stack+0x4d/0x6e
[   44.235461]  [] __warn+0xd1/0xf0
[   44.235464]  [] ? i915_pm_suspend_late+0x30/0x30
[   44.235465]  [] warn_slowpath_fmt+0x4f/0x60
[   44.235468]  [] ? pm_runtime_get_if_in_use+0x6e/0xa0
[   44.235469]  [] intel_runtime_pm_get_if_in_use+0xe6/0xf0
[   44.235471]  [] i915_gem_shrink+0x306/0x360
[   44.235473]  [] ? pci_platform_power_transition+0x24/0x90
[   44.235475]  [] ? i915_pm_suspend_late+0x30/0x30
[   44.235476]  [] i915_gem_shrink_all+0x1b/0x30
[   44.235478]  [] i915_gem_freeze_late+0x33/0x90
[   44.235479]  [] i915_pm_freeze_late+0x37/0x40
[   44.235481]  [] dpm_run_callback+0x4e/0x130
[   44.235483]  [] __device_suspend_late+0xdb/0x1f0
[   44.235484]  [] async_suspend_late+0x1f/0xa0
[   44.235486]  [] async_run_entry_fn+0x37/0x150
[   44.235488]  [] process_one_work+0x148/0x3f0
[   44.235490]  [] worker_thread+0x12b/0x490
[   44.235491]  [] ? process_one_work+0x3f0/0x3f0
[   44.235492]  [] kthread+0xc9/0xe0
[   44.235495]  [] ret_from_fork+0x1f/0x40
[   44.235496]  [] ? kthread_park+0x60/0x60
[   44.235497] ---[ end trace e438706b97c7f132 ]---

Alternatively, to actually shrink everything we have to do so slightly
earlier in the hibernation process.

To keep lockdep silent, we need to take struct_mutex for the shrinker
even though we know that we are the only user during the freeze.

Fixes: 7aab2d534e35 ("drm/i915: Shrink objects prior to hibernation")
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Reviewed-by: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_drv.c | 11 ++-
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 17 -
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8ae5853ea3c6..f2d0801c590c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1863,7 +1863,16 @@ static int i915_pm_resume(struct device *kdev)
 /* freeze: before creating the hibernation_image */
 static int i915_pm_freeze(struct device *kdev)
 {
-   return i915_pm_suspend(kdev);
+   int ret;
+   ret = i915_pm_suspend(kdev);
+   if (ret)
+   return ret;
+
+   ret = i915_gem_freeze(kdev_to_i915(kdev));
+   if (ret)
+   return ret;
+
+   return 0;
 }
 
 static int i915_pm_freeze_late(struct device *kdev)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 356899543d4b..008c74bfabad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3076,6 +3076,7 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void 
*data,
 void i915_gem_load_init(struct drm_device *dev);
 void i915_gem_load_cleanup(struct drm_device *dev);
 void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
+int i915_gem_freeze(struct drm_i915_private *dev_priv);
 int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
 
 void *i915_gem_object_alloc(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index aeb46658ab3c..1418c1c522cb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4593,6 +4593,19 @@ void 

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Queue page flip work with high priority (rev3)

[Imre Deak]

On ti, 2016-09-20 at 12:24 +, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Queue page flip work with high priority (rev3)
> URL   : https://patchwork.freedesktop.org/series/12336/
> State : warning
> 
> == Summary ==
> 
> Series 12336v3 drm/i915: Queue page flip work with high priority
> https://patchwork.freedesktop.org/api/1.0/series/12336/revisions/3/mb
> ox/
> 
> Test gem_exec_gttfill:
> Subgroup basic:
> pass   -> SKIP   (fi-snb-2600)

"""
Estimated that we need 513 objects and 2053 MiB for the test, but only
have 2013 MiB available (RAM) and a maximum of 383430 objects
Last errno: 22, Invalid argument
Subtest basic: SKIP (0.000s)
"""

It's a pre-existing sporadic issue on multiple platforms.

> Test gem_exec_suspend:
> Subgroup basic-s3:
> incomplete -> PASS   (fi-hsw-4770k)
> Test kms_pipe_crc_basic:
> Subgroup read-crc-pipe-c-frame-sequence:
> pass   -> SKIP   (fi-hsw-4770r)

"""
read-crc-pipe-C-frame-sequence: Testing connector HDMI-A-1 using pipe C
Test requirement not met in function test_read_crc, file
kms_pipe_crc_basic.c:178:
Test requirement: valid_connectors
No connector found for pipe 2
Subtest read-crc-pipe-C-frame-sequence: SKIP (0.179s)
"""

> Subgroup suspend-read-crc-pipe-a:
> pass   -> SKIP   (fi-hsw-4770r)

"""
suspend-read-crc-pipe-A: Testing connector HDMI-A-1 using pipe A
Test requirement not met in function test_read_crc, file
kms_pipe_crc_basic.c:178:
Test requirement: valid_connectors
No connector found for pipe 0
"""

In both cases above HDMI detect fails for outputs successfully detected
in earlier subtests, a pre-existing issue on multiple platforms.

> Subgroup suspend-read-crc-pipe-b:
> skip   -> PASS   (fi-hsw-4770r)
> Test kms_psr_sink_crc:
> Subgroup psr_basic:
> fail   -> DMESG-WARN (fi-skl-6700hq)

[drm:intel_pipe_update_start] *ERROR* Potential atomic update failure on pipe A

Pre-existing issue on this machine (and the test succeeded only 
once in CI history).

Pushed to -dinq, thanks for the reviews.

--Imre

> 
> fi-bdw-
> 5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
> fi-bsw-
> n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
> fi-byt-
> n2820 total:244  pass:208  dwarn:0   dfail:0   fail:1   skip:35 
> fi-hsw-
> 4770k total:208  pass:187  dwarn:0   dfail:0   fail:0   skip:20 
> fi-hsw-
> 4770r total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:24 
> fi-ilk-
> 650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
> fi-ivb-
> 3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
> fi-ivb-
> 3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> fi-skl-
> 6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
> fi-skl-
> 6700hqtotal:244  pass:221  dwarn:1   dfail:0   fail:0   skip:22 
> fi-skl-
> 6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
> fi-skl-
> 6770hqtotal:244  pass:228  dwarn:1   dfail:0   fail:1   skip:14 
> fi-snb-
> 2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
> fi-snb-
> 2600  total:244  pass:206  dwarn:0   dfail:0   fail:0   skip:38 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_2560/
> 
> de934d54f765dcfa82c018ea5d256801cffee7af drm-intel-nightly: 2016y-
> 09m-20d-09h-13m-55s UTC integration manifest
> 04838a2 drm/i915: Queue page flip work via a low latency, unbound
> workqueue
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 1/9] drm/i915/skl: pass pipe_wm in skl_compute_(wm_level/plane_wm) functions

[Mahesh Kumar]

there are two main reason I want to pass complete pipe_wm to compute 
function


1. for transition WM calculation, else we need to pass 
trans_wm.plane_res_b &  trans_wm.plane_res_l also as a parameter,


that will increase the parameter-list. And trans_wm will be used 
(calculation will happen) only during level-0. All other levels (1-7) 
these will be extra unused variables.


If we want to calculate transition WM separately (after plane WM 
calculation) there we will have to duplicate some of the code for WM 
level-0 calculation.


2. This will be used in Render compression WA for (Gen9 & CNL) where we 
have to make sure WM level-(1-7) result blocks/lines are >= WM level-0 
result blocks/lines.



agree, out_ variable should not be local variables.
If you think above points can be addressed, let me know, will update 
patches accordingly.


thanks,
-Mahesh

On Tuesday 20 September 2016 05:47 PM, Paulo Zanoni wrote:

Em Sex, 2016-09-09 às 13:30 +0530, Kumar, Mahesh escreveu:

From: Mahesh Kumar 

This patch make use of plane_wm variable directly instead of passing
skl_plane_wm struct. this way reduces number of argument requirement
in watermark calculation functions.

It also gives more freedom of decision making to implement Bspec WM
workarounds.

This is just my personal opinion, but I don't think this patch is an
improvement to the codebase. The way things are currently organized,
there's no risk that we may end up reading some variable that's still
not set/computed, so there's less risk that some later patch may end up
using information it shouldn't. Also, by having these explicit out
parameters, we clearly highlight what's the goal of the function:
output those 3 values, instead of writing I-don't-know-what to a huge
struct.

Besides, the patch even keeps the out_ variables as local variables
instead of parameters, which makes things even more confusing IMHO,
since in_ and out_ variables are usually function parameters.

I also see that this patch is not necessary for the series. We kinda
use the new pipe_wm variable at patch 9, but we could just go with the
variables we have.

So, unless some new arguments are given, I'd suggest to just drop this
patch.


Signed-off-by: Mahesh Kumar 
---
  drivers/gpu/drm/i915/intel_pm.c | 29 +++--
  1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 86c6d66..3fdec4d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3542,9 +3542,7 @@ static int skl_compute_plane_wm(const struct
drm_i915_private *dev_priv,
struct intel_plane_state
*intel_pstate,
uint16_t ddb_allocation,
int level,
-   uint16_t *out_blocks, /* out */
-   uint8_t *out_lines, /* out */
-   bool *enabled /* out */)
+   struct skl_pipe_wm *pipe_wm)
  {
struct drm_plane_state *pstate = _pstate->base;
struct drm_framebuffer *fb = pstate->fb;
@@ -3557,6 +3555,11 @@ static int skl_compute_plane_wm(const struct
drm_i915_private *dev_priv,
uint32_t width = 0, height = 0;
uint32_t plane_pixel_rate;
uint32_t y_tile_minimum, y_min_scanlines;
+   int id = skl_wm_plane_id(to_intel_plane(pstate->plane));
+   struct skl_wm_level *result = _wm->wm[level];
+   uint16_t *out_blocks = >plane_res_b[id];
+   uint8_t *out_lines = >plane_res_l[id];
+   bool *enabled = >plane_en[id];
  
  	if (latency == 0 || !cstate->base.active || !intel_pstate-

base.visible) {

*enabled = false;
@@ -3673,7 +3676,7 @@ skl_compute_wm_level(const struct
drm_i915_private *dev_priv,
 struct skl_ddb_allocation *ddb,
 struct intel_crtc_state *cstate,
 int level,
-struct skl_wm_level *result)
+struct skl_pipe_wm *pipe_wm)
  {
struct drm_atomic_state *state = cstate->base.state;
struct intel_crtc *intel_crtc = to_intel_crtc(cstate-

base.crtc);

@@ -3684,12 +3687,6 @@ skl_compute_wm_level(const struct
drm_i915_private *dev_priv,
enum pipe pipe = intel_crtc->pipe;
int ret;
  
-	/*

-* We'll only calculate watermarks for planes that are
actually
-* enabled, so make sure all other planes are set as
disabled.
-*/
-   memset(result, 0, sizeof(*result));
-
for_each_intel_plane_mask(_priv->drm,
  intel_plane,
  cstate->base.plane_mask) {
@@ -3727,9 +3724,7 @@ skl_compute_wm_level(const struct
drm_i915_private *dev_priv,
   intel_pstate,
   ddb_blocks,
   

Re: [Intel-gfx] [PATCH v4] drm/i915/skl: New ddb allocation algorithm

[Mahesh Kumar]

Hi Maarten,

thanks for pointing out the issue,

not only start value, end value is also incorrect. I got the root-cause 
for both.
but this end value issue seems to be always there. end value should be 
-1 (because start block should also be counted)

Will fix both & upload the reworked patches.

-Mahesh


On Monday 19 September 2016 03:25 PM, Maarten Lankhorst wrote:

Op 14-09-16 om 14:36 schreef Mahesh Kumar:

Hi,
There was an issue with transition WM, it was getting enabled & causing fifo 
underrun.
I fixed the condition, After that tested kms_plane & not getting any underrun.
Please let me know if you see any other issue.

Regards,
-Mahesh

On Tuesday 13 September 2016 06:10 PM, Maarten Lankhorst wrote:

Op 13-09-16 om 14:15 schreef Kumar, Mahesh:

From: Mahesh Kumar 

This patch implements new DDB allocation algorithm as per HW team
suggestion. This algo takecare of scenario where we allocate less DDB
for the planes with lower relative pixel rate, but they require more DDB
to work.
It also takes care of enabling same watermark level for each
plane, for efficient power saving.

Changes since v1:
   - Rebase on top of Paulo's patch series

Changes since v2:
   - Fix the for loop condition to enable WM

Signed-off-by: Mahesh Kumar 

I'm still getting underrun issues when running the entire patch series against 
kms_atomic_transition and kms_plane.
Can you confirm?

~Maarten

Found it..

During the test run:
# cat /sys/kernel/debug/dri/0/i915_ddb_info
   Start EndSize
Pipe A
   Plane1  0   0   0
   Plane2 30 890 860
   Cursor860 892  32

Pretty sure the start value here is bogus, and plane2 wm's end up overlapping 
with cursor.

~Maarten



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [v2,RESEND,1/2] drm/i915: Cleanup instdone collection

[Imre Deak]

On ti, 2016-09-20 at 14:23 +, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v2,RESEND,1/2] drm/i915: Cleanup
> instdone collection
> URL   : https://patchwork.freedesktop.org/series/12714/
> State : warning
> 
> == Summary ==
> 
> Series 12714v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/12714/revisions/1/mb
> ox/
> 
> Test drv_module_reload_basic:
> pass   -> SKIP   (fi-skl-6770hq)

"""
Reloading i915.ko with
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
module successfully unloaded
module successfully loaded again
Reloading i915.ko with inject_load_failure=1
unbinding /sys/class/vtconsole/vtcon1/: (M) frame buffer device
rmmod: ERROR: Module i915 is in use
"""

Pre-existing sporadic issue on multiple platforms, after a successful
first module reload.

> Test kms_pipe_crc_basic:
> Subgroup hang-read-crc-pipe-a:
> pass   -> SKIP   (fi-hsw-4770r)

"""
hang-read-crc-pipe-A: Testing connector HDMI-A-1 using pipe A
Test requirement not met in function test_read_crc, file
kms_pipe_crc_basic.c:178:
Test requirement: valid_connectors
No connector found for pipe 0
"""

Pre-existing sporadic issue, HDMI detect fails for an output that was
present in earlier subtests.

Thanks for the patches and review, I pushed them to -dinq.

--Imre


> Test kms_psr_sink_crc:
> Subgroup psr_basic:
> dmesg-warn -> PASS   (fi-skl-6700hq)
> 
> fi-bdw-
> 5557u total:244  pass:229  dwarn:0   dfail:0   fail:0   skip:15 
> fi-bsw-
> n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
> fi-hsw-
> 4770k total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
> fi-hsw-
> 4770r total:244  pass:221  dwarn:0   dfail:0   fail:0   skip:23 
> fi-ilk-
> 650   total:244  pass:182  dwarn:0   dfail:0   fail:2   skip:60 
> fi-ivb-
> 3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
> fi-ivb-
> 3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> fi-skl-
> 6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
> fi-skl-
> 6700hqtotal:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
> fi-skl-
> 6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
> fi-skl-
> 6770hqtotal:244  pass:227  dwarn:1   dfail:0   fail:1   skip:15 
> fi-snb-
> 2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
> fi-snb-
> 2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_2561/
> 
> 4ca90e7c3b6e429e033b93fc56fc156da8f222ef drm-intel-nightly: 2016y-
> 09m-20d-12h-43m-32s UTC integration manifest
> 4721d61 drm/i915: Try to print INSTDONE bits for all slice/subslice
> 6240439 drm/i915: Cleanup instdone collection
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/38] drm/i915: Markup GEM API with lockdep asserts

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> Add lockdep_assert_held(struct_mutex) to the API preamble of the
> internal GEM interfaces.
> 

Heck yeah!

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 11/38] drm/i915: Introduce an internal allocator for disposable private objects

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> Quite a few of our objects used for internal hardware programming do not
> benefit from being swappable or from being zero initialised. As such
> they do not benefit from using a shmemfs backing storage and since they
> are internal and never directly exposed to the user, we do not need to
> worry about providing a filp. For these we can use an
> drm_i915_gem_object wrapper around a sg_table of plain struct page. They
> are not swapped backed and not automatically pinned. If they are reaped

"swap backed"

> by the shrinker, the pages are released and the contents discarded. For
> the internal use case, this is fine as for example, ringbuffers are
> pinned from being written by a request to be read by the hardware. Once
> they are idle, they can be discarded entirely. As such they are a good
> match for execlist ringbuffers and a small varierty of other internal
> objects.
> 
> In the first iteration, this is limited to the scratch batch buffers we
> use (for command parsing and state inaitialisation).
> 
> Signed-off-by: Chris Wilson 

> +++ b/drivers/gpu/drm/i915/i915_gem_internal.c
> @@ -0,0 +1,156 @@
> +/*
> + * Copyright © 2015 Intel Corporation

2016 already.

> +#include 
> +#include 
> +#include "i915_drv.h"
> +
> +static void __i915_gem_object_free_pages(struct sg_table *st)

Name makes me feel like this was be misplaced here. Maybe just
__object_free_pages or similar.

> +static int i915_gem_object_get_pages_internal(struct drm_i915_gem_object 
> *obj)
> +{

Much copied from i915_gem_object_get_pages_gtt, not sure if there's
place for code reuse. Copy paste and modify not my favourite.

> +}
> +
> +static void i915_gem_object_put_pages_internal(struct drm_i915_gem_object 
> *obj)
> +{
> + __i915_gem_object_free_pages(obj->pages);
> +
> + obj->dirty = 0;
> + obj->madv = I915_MADV_WILLNEED;

This seems so backwards it might be worth a comment. It's written kind
of inverted just to dodge the usual code paths, right? I'm not sure if
it helps the code readability.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v12 5/7] drm/i915/skl: Ensure pipes with changed wms get added to the state

[Mike Lothian]

I've raised https://bugs.freedesktop.org/show_bug.cgi?id=97888 I'll
attach the info you requested once I get back to my machine

On 21 September 2016 at 07:56, Maarten Lankhorst
 wrote:
> Hey,
>
> Op 20-09-16 om 20:45 schreef Mike Lothian:
>> Hi
>>
>> I've bisected back to this commit in the drm-intel-nightly branch
>>
>> 05a76d3d6ad1ee9f9814f88949cc9305fc165460 is the first bad commit
>> commit 05a76d3d6ad1ee9f9814f88949cc9305fc165460
>> Author: Lyude 
>> Date:   Wed Aug 17 15:55:57 2016 -0400
>>
>>drm/i915/skl: Ensure pipes with changed wms get added to the state
>>
>>If we're enabling a pipe, we'll need to modify the watermarks on all
>>active planes. Since those planes won't be added to the state on
>>their own, we need to add them ourselves.
>>
>>Signed-off-by: Lyude 
>>Reviewed-by: Matt Roper 
>>Cc: sta...@vger.kernel.org
>>Cc: Ville Syrjälä 
>>Cc: Daniel Vetter 
>>Cc: Radhakrishna Sripada 
>>Cc: Hans de Goede 
>>Signed-off-by: Maarten Lankhorst 
>>Link: 
>> http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-6-git-send-email-cp...@redhat.com
>>
>> The symptoms I'm seeing look like tearing at the top of the screen and
>> it's especially noticeable in Chrome - reverting this commit makes the
>> issue go away
>>
>> Let me know if you'd like me to raise a bug
> Please do so, it's nice to refer to when making a fix for it.
>
> Could you attach the contents of /sys/kernel/debug/dri/0/i915_ddb_info for 
> working and not-working in it?
>
> ~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Fix typo in encoder docs

[Daniel Vetter]

On Mon, Sep 19, 2016 at 03:40:48PM -0700, Dhinakaran Pandiyan wrote:
> Corrected typo in bridge and encoder comparison. Also, added a one-line
> encoder description from the previous documentation.
> 
> Cc: Daniel Vetter 
> Cc: Archit Taneja 
> 
> Signed-off-by: Dhinakaran Pandiyan 

Thanks for spotting these and creating the patch, applied to drm-misc.
-Daniel

> ---
>  drivers/gpu/drm/drm_encoder.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 998a674..5c06771 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -31,20 +31,21 @@
>   *
>   * Encoders represent the connecting element between the CRTC (as the overall
>   * pixel pipeline, represented by struct _crtc) and the connectors (as 
> the
> - * generic sink entity, represented by struct _connector). Encoders are
> - * objects exposed to userspace, originally to allow userspace to infer 
> cloning
> - * and connector/CRTC restrictions. Unfortunately almost all drivers get this
> - * wrong, making the uabi pretty much useless. On top of that the exposed
> - * restrictions are too simple for todays hardware, and the recommend way to
> - * infer restrictions is by using the DRM_MODE_ATOMIC_TEST_ONLY flag for the
> - * atomic IOCTL.
> + * generic sink entity, represented by struct _connector). An encoder 
> takes
> + * pixel data from a CRTC and converts it to a format suitable for any 
> attached
> + * connector. Encoders are objects exposed to userspace, originally to allow
> + * userspace to infer cloning and connector/CRTC restrictions. Unfortunately
> + * almost all drivers get this wrong, making the uabi pretty much useless. On
> + * top of that the exposed restrictions are too simple for today's hardware, 
> and
> + * the recommended way to infer restrictions is by using the
> + * DRM_MODE_ATOMIC_TEST_ONLY flag for the atomic IOCTL.
>   *
>   * Otherwise encoders aren't used in the uapi at all (any modeset request 
> from
>   * userspace directly connects a connector with a CRTC), drivers are 
> therefore
>   * free to use them however they wish. Modeset helper libraries make strong 
> use
>   * of encoders to facilitate code sharing. But for more complex settings it 
> is
>   * usually better to move shared code into a separate _bridge. Compared 
> to
> - * encoders bridges also have the benefit of not being purely an internal
> + * encoders, bridges also have the benefit of being purely an internal
>   * abstraction since they are not exposed to userspace at all.
>   *
>   * Encoders are initialized with drm_encoder_init() and cleaned up using
> -- 
> 2.5.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 10/38] drm/i915: Defer active reference until required

[Joonas Lahtinen]

Kerneldoc could help with getting grip of the convention introduced
here.

Does this really have visible results in real world workloads? It adds
further complexity.

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> @@ -2383,6 +2390,28 @@ i915_gem_object_has_active_engine(const struct 
> drm_i915_gem_object *obj,
>   return obj->flags & BIT(engine + I915_BO_ACTIVE_SHIFT);
>  }
>  
> +static inline bool
> +i915_gem_object_has_active_reference(const struct drm_i915_gem_object *obj)
> +{
> + return obj->flags & I915_BO_ACTIVE_REF;
> +}
> 

We're still having a mix of set_bit and manual fondling. Oh well.

> @@ -2413,6 +2442,11 @@ static inline void i915_vma_put(struct i915_vma *vma)
> >     i915_gem_object_put(vma->obj);
>  }
>  
> +static inline void i915_vma_put_internal(struct i915_vma *vma)

Not __i915_vma_put? We only have one _internal function, lets not add
another style mixing here.
 
> +void __i915_gem_object_release_unless_active(struct drm_i915_gem_object *obj)
> +{
> + if (!obj)
> + return;

I know I hate this.

Anyway,

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/38] drm/i915: Rearrange i915_wait_request() accounting with callers

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> Our low-level wait routine has evolved from our generic wait interface
> that handled unlocked, RPS boosting, waits with time tracking. If we
> push our GEM fence tracking to use reservation_objects (required for
> handling multiple timelines), we lose the ability to pass the required
> information down to i915_wait_request(). However, if we push the extra
> functionality from i915_wait_request() to the individual callsites
> (i915_gem_object_wait_rendering and i915_gem_wait_ioctl) that make use
> of those extras, we can both simplify our low level wait and prepare for
> extending the GEM interface for use of reservation_objects.
> 
> * This causes a temporary regression in the use of wait-ioctl as a busy
> query -- it will fail to report immediately, but go into
> i915_wait_request() before timing out.
> 
> Signed-off-by: Chris Wilson 

Already provided review comments in the previous series, no changelog
and at least the two first were not accounted for.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/38] drm/i915: Support asynchronous waits on struct fence from i915_gem_request

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> + if (!fence_is_array(fence)) {
> + ret = i915_sw_fence_await_dma_fence(>submit,
> + fence, 10*HZ,

#define I915_MAX_USERSPACE_WAIT or something. We already have two
instances of it with only one explanation. Like mentinoed in the
previous review.

> + GFP_KERNEL);
> + return ret < 0 ? ret : 0;
> + }
> +

With the #define added;

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/38] drm/i915: Compress GPU objects in error state

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> @@ -175,6 +176,110 @@ static void i915_error_puts(struct 
> drm_i915_error_state_buf *e,
>  #define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
>  #define err_puts(e, s) i915_error_puts(e, s)
>  
> +#ifdef CONFIG_DRM_I915_COMPRESS_ERROR
> +
> +static bool compress_init(struct z_stream_s *zstream)
> +{
> + memset(zstream, 0, sizeof(*zstream));
> +
> + zstream->workspace =
> + kmalloc(zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL),
> + GFP_ATOMIC | __GFP_NOWARN);
> + if (!zstream->workspace)
> + return NULL;

return false;

> +static int compress_page(struct z_stream_s *zstream,
> +  void *src,
> +  struct drm_i915_error_object *dst)
> +{
> + zstream->next_in = src;
> + zstream->avail_in = PAGE_SIZE;
> +
> + do {
> + if (zstream->avail_out == 0) {
> + unsigned long page;
> +
> + page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
> + if (!page)
> + return -ENOMEM;
> +
> + dst->pages[dst->page_count++] = (void *)page;
> +
> + zstream->next_out = (void *)page;
> + zstream->avail_out = PAGE_SIZE;
> + }
> +
> + if (zlib_deflate(zstream, Z_SYNC_FLUSH) != Z_OK)
> + return -EIO;
> +
> + /* Fallback to uncompressed if we increase size? */
> + if (0 && zstream->total_out > zstream->total_in)
> + return -E2BIG;

We could still end up decreasing in the future, so this check should
really be outside of this function at the end of compression.

> @@ -327,13 +450,23 @@ static void print_error_obj(struct 
> drm_i915_error_state_buf *m,
>      lower_32_bits(obj->gtt_offset));
>   }
>  
> - for (page = offset = 0; page < obj->page_count; page++) {
> - for (elt = 0; elt < PAGE_SIZE/4; elt++) {
> - err_printf(m, "%08x :  %08x\n", offset,
> -    obj->pages[page][elt]);
> - offset += 4;
> + err_compression_marker(m);
> + for (page = 0; page < obj->page_count; page++) {
> + int i, len;
> +
> + len = PAGE_SIZE;
> + if (page == obj->page_count - 1)
> + len -= obj->unused;
> + len = (len + 3) / 4;

Magic-ish. ascii85_length() or so.

> + num_pages = DIV_ROUND_UP(10 * num_pages, 8); /* worstcase zlib growth */

Could be a function added to kernel zlib.

Above addressed;

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/10] drm: Extract drm_plane.[hc]

[Sean Paul]

On Mon, Sep 19, 2016 at 4:11 PM, Daniel Vetter  wrote:
> On Tue, Sep 06, 2016 at 12:59:31PM -0400, Sean Paul wrote:
>> On Wed, Aug 31, 2016 at 12:09 PM, Daniel Vetter  
>> wrote:
>> > Just pure code movement, cleanup and polish will happen in later
>> > patches.
>> >
>> > v2: Don't forget all the ioctl! To extract those cleanly I decided to
>> > put check_src_coords into drm_framebuffer.c (and give it a
>> > drm_framebuffer_ prefix), since that just checks framebuffer
>> > constraints.
>> >
>> > Signed-off-by: Daniel Vetter 
>> > ---
>> >  Documentation/gpu/drm-kms.rst   |  12 +
>> >  drivers/gpu/drm/Makefile|   3 +-
>> >  drivers/gpu/drm/drm_crtc.c  | 939 
>> > +---
>> >  drivers/gpu/drm/drm_crtc_internal.h |  38 +-
>> >  drivers/gpu/drm/drm_framebuffer.c   |  26 +
>> >  drivers/gpu/drm/drm_plane.c | 937 
>> > +++
>> >  include/drm/drm_atomic.h| 154 ++
>> >  include/drm/drm_crtc.h  | 583 +-
>> >  include/drm/drm_plane.h | 470 ++
>> >  9 files changed, 1628 insertions(+), 1534 deletions(-)
>> >  create mode 100644 drivers/gpu/drm/drm_plane.c
>> >  create mode 100644 include/drm/drm_plane.h
>> >
>> > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
>> > index f9a991bb87d4..33181be97151 100644
>> > --- a/Documentation/gpu/drm-kms.rst
>> > +++ b/Documentation/gpu/drm-kms.rst
>> > @@ -110,6 +110,18 @@ Note that dumb objects may not be used for gpu 
>> > acceleration, as has been
>> >  attempted on some ARM embedded platforms. Such drivers really must have
>> >  a hardware-specific ioctl to allocate suitable buffer objects.
>> >
>> > +Plane Abstraction
>> > +=
>> > +
>> > +Plane Functions Reference
>> > +-
>> > +
>> > +.. kernel-doc:: include/drm/drm_plane.h
>> > +   :internal:
>> > +
>> > +.. kernel-doc:: drivers/gpu/drm/drm_plane.c
>> > +   :export:
>> > +
>> >  Display Modes Function Reference
>> >  
>> >
>> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> > index 439d89b25ae0..8eeb07a35798 100644
>> > --- a/drivers/gpu/drm/Makefile
>> > +++ b/drivers/gpu/drm/Makefile
>> > @@ -14,7 +14,8 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
>> > drm_rect.o drm_vma_manager.o drm_flip_work.o \
>> > drm_modeset_lock.o drm_atomic.o drm_bridge.o \
>> > drm_framebuffer.o drm_connector.o drm_blend.o \
>> > -   drm_encoder.o drm_mode_object.o drm_property.o
>> > +   drm_encoder.o drm_mode_object.o drm_property.o \
>> > +   drm_plane.o
>> >
>> >  drm-$(CONFIG_COMPAT) += drm_ioc32.o
>> >  drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
>> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> > index 0fad433f4d2d..513ab4729683 100644
>> > --- a/drivers/gpu/drm/drm_crtc.c
>> > +++ b/drivers/gpu/drm/drm_crtc.c
>>
>> 
>>
>> > -
>> > -static int check_src_coords(uint32_t src_x, uint32_t src_y,
>> > -   uint32_t src_w, uint32_t src_h,
>> > -   const struct drm_framebuffer *fb)
>> > -{
>> > -   unsigned int fb_width, fb_height;
>> > -
>> > -   fb_width = fb->width << 16;
>> > -   fb_height = fb->height << 16;
>> > -
>> > -   /* Make sure source coordinates are inside the fb. */
>> > -   if (src_w > fb_width ||
>> > -   src_x > fb_width - src_w ||
>> > -   src_h > fb_height ||
>> > -   src_y > fb_height - src_h) {
>> > -   DRM_DEBUG_KMS("Invalid source coordinates "
>> > - "%u.%06ux%u.%06u+%u.%06u+%u.%06u\n",
>> > - src_w >> 16, ((src_w & 0x) * 15625) >> 
>> > 10,
>> > - src_h >> 16, ((src_h & 0x) * 15625) >> 
>> > 10,
>> > - src_x >> 16, ((src_x & 0x) * 15625) >> 
>> > 10,
>> > - src_y >> 16, ((src_y & 0x) * 15625) >> 
>> > 10);
>> > -   return -ENOSPC;
>> > -   }
>> > -
>> > -   return 0;
>> > -}
>>
>> I'm good with this change, but I'd argue that it probably belongs in
>> its own patch.
>
> Except for moving the function + giving it a prefix (since it's no longer
> static) there's no change here.

That's fair.

>>
>>
>> 
>>
>> >  /**
>> > - * drm_mode_page_flip_ioctl - schedule an asynchronous fb update
>> > - * @dev: DRM device
>> > - * @data: ioctl data
>> > - * @file_priv: DRM file info
>> > - *
>> > - * This schedules an asynchronous update on a given CRTC, called page 
>> > flip.
>> > - * Optionally a drm event is generated to signal the completion of the 
>> > event.
>> > - * Generic drivers cannot assume that a pageflip with changed framebuffer
>> > - * properties 

Re: [Intel-gfx] [PATCH 25/38] drm: Add reference counting to drm_atomic_state

[Sean Paul]

On Tue, Sep 20, 2016 at 11:29 AM, Chris Wilson  wrote:
> drm_atomic_state has a complicated single owner model that tracks the
> single reference from allocation through to destruction on another
> thread - or perhaps on a local error path. We can simplify this tracking
> by using reference counting (at a cost of a few more atomics). This is
> even more beneficial when the lifetime of the state becomes more
> convoluted than being passed to a single worker thread for the commit.
>
> v2: Double check !intel atomic_commit functions for missing gets
> v3: Update kerneldocs
>
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: dri-de...@lists.freedesktop.org
> Reviewed-by: Eric Engestrom 

This looks good to me, it'll hopefully give drivers a bit more
flexibility with respect to keeping state around during/directly after
commit.

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  3 +-
>  drivers/gpu/drm/drm_atomic.c | 25 +++
>  drivers/gpu/drm/drm_atomic_helper.c  | 98 
> +++-
>  drivers/gpu/drm/drm_fb_helper.c  |  9 +--
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |  3 +-
>  drivers/gpu/drm/i915/i915_debugfs.c  |  3 +-
>  drivers/gpu/drm/i915/intel_display.c | 31 +
>  drivers/gpu/drm/i915/intel_sprite.c  |  4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c   |  3 +-
>  drivers/gpu/drm/msm/msm_atomic.c |  3 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c   |  3 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c|  3 +-
>  drivers/gpu/drm/sti/sti_drv.c|  3 +-
>  drivers/gpu/drm/tegra/drm.c  |  3 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  2 -
>  drivers/gpu/drm/vc4/vc4_kms.c|  3 +-
>  include/drm/drm_atomic.h | 28 +++-
>  include/drm/drm_crtc.h   |  3 +
>  18 files changed, 101 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 8e7483d90c47..c9e645fa1233 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -464,7 +464,7 @@ atmel_hlcdc_dc_atomic_complete(struct 
> atmel_hlcdc_dc_commit *commit)
>
> drm_atomic_helper_cleanup_planes(dev, old_state);
>
> -   drm_atomic_state_free(old_state);
> +   drm_atomic_state_put(old_state);
>
> /* Complete the commit, wake up any waiter. */
> spin_lock(>commit.wait.lock);
> @@ -521,6 +521,7 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device 
> *dev,
> /* Swap the state, this is the point of no return. */
> drm_atomic_helper_swap_state(state, true);
>
> +   drm_atomic_state_get(state);
> if (async)
> queue_work(dc->wq, >work);
> else
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 23739609427d..5dd70540219c 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -74,6 +74,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_release);
>  int
>  drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state)
>  {
> +   kref_init(>ref);
> +
> /* TODO legacy paths should maybe do a better job about
>  * setting this appropriately?
>  */
> @@ -215,22 +217,16 @@ void drm_atomic_state_clear(struct drm_atomic_state 
> *state)
>  EXPORT_SYMBOL(drm_atomic_state_clear);
>
>  /**
> - * drm_atomic_state_free - free all memory for an atomic state
> - * @state: atomic state to deallocate
> + * __drm_atomic_state_free - free all memory for an atomic state
> + * @ref: This atomic state to deallocate
>   *
>   * This frees all memory associated with an atomic state, including all the
>   * per-object state for planes, crtcs and connectors.
>   */
> -void drm_atomic_state_free(struct drm_atomic_state *state)
> +void __drm_atomic_state_free(struct kref *ref)
>  {
> -   struct drm_device *dev;
> -   struct drm_mode_config *config;
> -
> -   if (!state)
> -   return;
> -
> -   dev = state->dev;
> -   config = >mode_config;
> +   struct drm_atomic_state *state = container_of(ref, typeof(*state), 
> ref);
> +   struct drm_mode_config *config = >dev->mode_config;
>
> drm_atomic_state_clear(state);
>
> @@ -243,7 +239,7 @@ void drm_atomic_state_free(struct drm_atomic_state *state)
> kfree(state);
> }
>  }
> -EXPORT_SYMBOL(drm_atomic_state_free);
> +EXPORT_SYMBOL(__drm_atomic_state_free);
>
>  /**
>   * drm_atomic_get_crtc_state - get crtc state
> @@ -1742,7 +1738,7 @@ retry:
> if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
> /*
>  * Unlike commit, check_only does not 

Re: [Intel-gfx] [PATCH 03/38] drm/i915: Always use the GTT for error capture

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote: 
> + /* Reserve a mappable slot for our lockless error capture */
> + ret = drm_mm_insert_node_in_range_generic(>base.mm,
> +   >gpu_error,

'error_page' would arguably be better name, or pretty much anything but
'gpu_error'.

> @@ -2788,6 +2802,9 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private 
> *dev_priv)
>  
>   i915_gem_cleanup_stolen(_priv->drm);
>  
> + if (drm_mm_node_allocated(>gpu_error))
> + drm_mm_remove_node(>gpu_error);
> +

Is there really a legit case of this being called twice? Or be called
without the init?

With the variable name fixed;

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v12 5/7] drm/i915/skl: Ensure pipes with changed wms get added to the state

[Mike Lothian]

Will do.

On Wed, 21 Sep 2016 at 07:56 Maarten Lankhorst <
maarten.lankho...@linux.intel.com> wrote:

> Hey,
>
> Op 20-09-16 om 20:45 schreef Mike Lothian:
> > Hi
> >
> > I've bisected back to this commit in the drm-intel-nightly branch
> >
> > 05a76d3d6ad1ee9f9814f88949cc9305fc165460 is the first bad commit
> > commit 05a76d3d6ad1ee9f9814f88949cc9305fc165460
> > Author: Lyude 
> > Date:   Wed Aug 17 15:55:57 2016 -0400
> >
> >drm/i915/skl: Ensure pipes with changed wms get added to the state
> >
> >If we're enabling a pipe, we'll need to modify the watermarks on all
> >active planes. Since those planes won't be added to the state on
> >their own, we need to add them ourselves.
> >
> >Signed-off-by: Lyude 
> >Reviewed-by: Matt Roper 
> >Cc: sta...@vger.kernel.org
> >Cc: Ville Syrjälä 
> >Cc: Daniel Vetter 
> >Cc: Radhakrishna Sripada 
> >Cc: Hans de Goede 
> >Signed-off-by: Maarten Lankhorst 
> >Link:
> http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-6-git-send-email-cp...@redhat.com
> >
> > The symptoms I'm seeing look like tearing at the top of the screen and
> > it's especially noticeable in Chrome - reverting this commit makes the
> > issue go away
> >
> > Let me know if you'd like me to raise a bug
> Please do so, it's nice to refer to when making a fix for it.
>
> Could you attach the contents of /sys/kernel/debug/dri/0/i915_ddb_info for
> working and not-working in it?
>
> ~Maarten
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v12 5/7] drm/i915/skl: Ensure pipes with changed wms get added to the state

[Maarten Lankhorst]

Hey,

Op 20-09-16 om 20:45 schreef Mike Lothian:
> Hi
>
> I've bisected back to this commit in the drm-intel-nightly branch
>
> 05a76d3d6ad1ee9f9814f88949cc9305fc165460 is the first bad commit
> commit 05a76d3d6ad1ee9f9814f88949cc9305fc165460
> Author: Lyude 
> Date:   Wed Aug 17 15:55:57 2016 -0400
>
>drm/i915/skl: Ensure pipes with changed wms get added to the state
>
>If we're enabling a pipe, we'll need to modify the watermarks on all
>active planes. Since those planes won't be added to the state on
>their own, we need to add them ourselves.
>
>Signed-off-by: Lyude 
>Reviewed-by: Matt Roper 
>Cc: sta...@vger.kernel.org
>Cc: Ville Syrjälä 
>Cc: Daniel Vetter 
>Cc: Radhakrishna Sripada 
>Cc: Hans de Goede 
>Signed-off-by: Maarten Lankhorst 
>Link: 
> http://patchwork.freedesktop.org/patch/msgid/1471463761-26796-6-git-send-email-cp...@redhat.com
>
> The symptoms I'm seeing look like tearing at the top of the screen and
> it's especially noticeable in Chrome - reverting this commit makes the
> issue go away
>
> Let me know if you'd like me to raise a bug
Please do so, it's nice to refer to when making a fix for it.

Could you attach the contents of /sys/kernel/debug/dri/0/i915_ddb_info for 
working and not-working in it?

~Maarten
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/10] drm/doc: Polish for drm_plane.[hc]

[Archit Taneja]

On 09/19/2016 06:43 PM, Daniel Vetter wrote:

On Fri, Sep 02, 2016 at 03:00:38PM +0530, Archit Taneja wrote:



On 8/31/2016 9:39 PM, Daniel Vetter wrote:

Big thing is untangling and carefully documenting the different uapi
types of planes. I also sprinkled a few more cross references around
to make this easier to discover.

As usual, remove the kerneldoc for internal functions which are not
exported. Aside: We should probably go OCD on all the ioctl handlers
and consistenly give them an _ioctl postfix.

Signed-off-by: Daniel Vetter 
---
  Documentation/gpu/drm-kms.rst |  47 +--
  drivers/gpu/drm/drm_crtc.c|   6 +-
  drivers/gpu/drm/drm_plane.c   | 132 --
  include/drm/drm_plane.h   |  57 +-
  4 files changed, 86 insertions(+), 156 deletions(-)





+/**
+ * enum drm_plane_type - uapi plane type enumeration
+ *
+ * For historical reasons not all planes are made the same. This enumeration is
+ * used to tell the different types of planes apart to implement the different
+ * uapi semantics for them. For userspace which is universal plane aware and
+ * which is using that atomic IOCTL there's no difference between these planes
+ * (beyong what the driver and hardware can support of course).
+ *
+ * For compatibility with legacy userspace, only overlay planes are made
+ * available to userspace by default. Userspace clients may set the
+ * DRM_CLIENT_CAP_UNIVERSAL_PLANES client capability bit to indicate that they
+ * wish to receive a universal plane list containing all plane types. See also
+ * drm_for_each_legacy_plane().
+ */
  enum drm_plane_type {
-   DRM_PLANE_TYPE_OVERLAY,


Any reason why you moved this down? I guess there is no harm, but people
might be printing plane type while debugging, and they'd assume
DRM_PLANE_TYPE_OVERLAY=0


I think starting out with 0 for the primary plane makes a lot more sense,
and since it's an internal thing we can change it however we want. I also
think from a documentation pov it reads better if the 2 special planes
(primary and cursor) are first.

But I'm happy to shuffle it back if you feel strongly the other way round.


No, it's fine. The series looks good too.

Thanks,
Archit


-Daniel



Thanks,
Archit


+   /**
+* @DRM_PLANE_TYPE_PRIMARY:
+*
+* Primary planes represent a "main" plane for a CRTC.  Primary planes
+* are the planes operated upon by CRTC modesetting and flipping
+* operations described in the page_flip and set_config hooks in struct
+* _crtc_funcs.
+*/
DRM_PLANE_TYPE_PRIMARY,
+
+   /**
+* @DRM_PLANE_TYPE_CURSOR:
+*
+* Cursor planes represent a "cursor" plane for a CRTC.  Cursor planes
+* are the planes operated upon by the DRM_IOCTL_MODE_CURSOR and
+* DRM_IOCTL_MODE_CURSOR2 IOCTLs.
+*/
DRM_PLANE_TYPE_CURSOR,
+
+   /**
+* @DRM_PLANE_TYPE_OVERLAY:
+*
+* Overlay planes represent all non-primary, non-cursor planes. Some
+* drivers refer to these types of planes as "sprites" internally.
+*/
+   DRM_PLANE_TYPE_OVERLAY,
  };


@@ -458,11 +496,26 @@ static inline struct drm_plane *drm_plane_find(struct 
drm_device *dev,
list_for_each_entry((plane), &(dev)->mode_config.plane_list, head) \
for_each_if ((plane_mask) & (1 << drm_plane_index(plane)))

-/* Plane list iterator for legacy (overlay only) planes. */
+/**
+ * drm_for_each_legacy_plane - iterate over all planes for legacy userspace
+ * @plane: the loop cursor
+ * @dev: the DRM device
+ *
+ * Iterate over all legacy planes of @dev, excluding primary and cursor planes.
+ * This is useful for implementing userspace apis when userspace is not
+ * universal plane aware. See also enum _plane_type.
+ */
  #define drm_for_each_legacy_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
for_each_if (plane->type == DRM_PLANE_TYPE_OVERLAY)

+/**
+ * drm_for_each_plane - iterate over all planes
+ * @plane: the loop cursor
+ * @dev: the DRM device
+ *
+ * Iterate over all planes of @dev, include primary and cursor planes.
+ */
  #define drm_for_each_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)




--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project




--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/38] drm/i915: Allow disabling error capture

[Joonas Lahtinen]

On ti, 2016-09-20 at 09:29 +0100, Chris Wilson wrote:
> We currently capture the GPU state after we detect a hang. This is vital
> for us to both triage and debug hangs in the wild (post-mortem
> debugging). However, it comes at the cost of running some potentially
> dangerous code (since it has to make very few assumption about the state
> of the driver) that is quite resource intensive.

I might have included more, like the member in the module parameter
structure, but I see you deliberately left them out.

Reviewed-by: Joonas Lahtinen 

Regards, Joonas 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx