[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Minor Fixes and Refactoring for HDMI PCON stuff (rev2)

2022-01-31 Thread Patchwork
== Series Details ==

Series: Minor Fixes and Refactoring for HDMI PCON stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/99311/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/ttm: Return some errors instead of trying memcpy move

2022-01-31 Thread Patchwork
== Series Details ==

Series: drm/i915/ttm: Return some errors instead of trying memcpy move
URL   : https://patchwork.freedesktop.org/series/99553/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11168 -> Patchwork_22145


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/index.html

Participating hosts (48 -> 41)
--

  Missing(7): fi-kbl-soraka fi-bdw-5557u shard-tglu fi-icl-u2 shard-rkl 
shard-dg1 fi-bdw-samus 

Known issues


  Here are the changes found in Patchwork_22145 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@i915_selftest@live@hangcheck:
- fi-hsw-4770:[PASS][1] -> [INCOMPLETE][2] ([i915#3303])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11168/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html

  * igt@kms_psr@primary_page_flip:
- fi-skl-6600u:   [PASS][3] -> [FAIL][4] ([i915#4547])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11168/fi-skl-6600u/igt@kms_psr@primary_page_flip.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/fi-skl-6600u/igt@kms_psr@primary_page_flip.html

  * igt@runner@aborted:
- fi-hsw-4770:NOTRUN -> [FAIL][5] ([fdo#109271] / [i915#1436] / 
[i915#4312])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/fi-hsw-4770/igt@run...@aborted.html
- fi-skl-6600u:   NOTRUN -> [FAIL][6] ([i915#4312])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/fi-skl-6600u/igt@run...@aborted.html

  
 Possible fixes 

  * igt@i915_selftest@live@gt_pm:
- {fi-jsl-1}: [DMESG-FAIL][7] ([i915#1886]) -> [PASS][8]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11168/fi-jsl-1/igt@i915_selftest@live@gt_pm.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/fi-jsl-1/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@hangcheck:
- bat-dg1-6:  [DMESG-FAIL][9] ([i915#4494] / [i915#4957]) -> 
[PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11168/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4898]: https://gitlab.freedesktop.org/drm/intel/issues/4898
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957


Build changes
-

  * Linux: CI_DRM_11168 -> Patchwork_22145

  CI-20190529: 20190529
  CI_DRM_11168: c2bd9b295e337f6a882ac5ec422171502090d33a @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6337: 7c9c034619ef9dbfbfe041fbf3973a1cf1ac7a22 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22145: 3d7fd168fedce25408645c7d4e38f19b3c832415 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3d7fd168fedc drm/i915/ttm: Return some errors instead of trying memcpy move

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22145/index.html


[Intel-gfx] [PATCH v2 4/4] drm/i915/display: Simplify helpers for getting DSC slices and bpp

2022-01-31 Thread Ankit Nautiyal
Genralize the helper for getting DSC slice count and compressed bpp
for HDMI sink supporting DSC.
This patch:
-Removes the assumption on the bpc and sends it as an argument for
calculating compressed bpc.
-Sends the resolution, and output format as parameters for which the
DSC paremeters are to be calculated instead of crtc_state.

v2: Added forward declaration for struct drm_display_mode.

Signed-off-by: Ankit Nautiyal 
---
 drivers/gpu/drm/i915/display/intel_dp.c   |  7 +--
 drivers/gpu/drm/i915/display/intel_hdmi.c | 24 ---
 drivers/gpu/drm/i915/display/intel_hdmi.h |  6 --
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index f7fe7de7e553..17d08f06499b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2357,7 +2357,9 @@ intel_dp_pcon_dsc_enc_slices(struct intel_dp *intel_dp,
int pcon_max_slices = 
drm_dp_pcon_dsc_max_slices(intel_dp->pcon_dsc_dpcd);
int pcon_max_slice_width = 
drm_dp_pcon_dsc_max_slice_width(intel_dp->pcon_dsc_dpcd);
 
-   return intel_hdmi_dsc_get_num_slices(crtc_state, pcon_max_slices,
+   return intel_hdmi_dsc_get_num_slices(&crtc_state->hw.adjusted_mode,
+crtc_state->output_format,
+pcon_max_slices,
 pcon_max_slice_width,
 hdmi_max_slices, hdmi_throughput);
 }
@@ -2374,9 +2376,10 @@ intel_dp_pcon_dsc_enc_bpp(struct intel_dp *intel_dp,
int pcon_fractional_bpp = 
drm_dp_pcon_dsc_bpp_incr(intel_dp->pcon_dsc_dpcd);
int hdmi_max_chunk_bytes =
connector->display_info.hdmi.dsc_cap.total_chunk_kbytes * 1024;
+   int bpc = crtc_state->pipe_bpp / 3;
 
return intel_hdmi_dsc_get_bpp(pcon_fractional_bpp, slice_width,
- num_slices, output_format, hdmi_all_bpp,
+ num_slices, output_format, bpc, 
hdmi_all_bpp,
  hdmi_max_chunk_bytes);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 381a9de3a015..f75e2384da63 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3004,7 +3004,8 @@ int intel_hdmi_dsc_get_slice_height(int vactive)
  * intel_hdmi_dsc_get_num_slices - get no. of dsc slices based on dsc encoder
  * and dsc decoder capabilities
  *
- * @crtc_state: intel crtc_state
+ * @mode: drm_display_mode for which num of slices are needed
+ * @output_format : pipe output format
  * @src_max_slices: maximum slices supported by the DSC encoder
  * @src_max_slice_width: maximum slice width supported by DSC encoder
  * @hdmi_max_slices: maximum slices supported by sink DSC decoder
@@ -3014,7 +3015,8 @@ int intel_hdmi_dsc_get_slice_height(int vactive)
  * and decoder.
  */
 int
-intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
+intel_hdmi_dsc_get_num_slices(const struct drm_display_mode *mode,
+ enum intel_output_format output_format,
  int src_max_slices, int src_max_slice_width,
  int hdmi_max_slices, int hdmi_throughput)
 {
@@ -3036,7 +3038,7 @@ intel_hdmi_dsc_get_num_slices(const struct 
intel_crtc_state *crtc_state,
int max_throughput; /* max clock freq. in khz per slice */
int max_slice_width;
int slice_width;
-   int pixel_clock = crtc_state->hw.adjusted_mode.crtc_clock;
+   int pixel_clock = mode->crtc_clock;
 
if (!hdmi_throughput)
return 0;
@@ -3047,8 +3049,8 @@ intel_hdmi_dsc_get_num_slices(const struct 
intel_crtc_state *crtc_state,
 * for 4:4:4 is 1.0. Multiplying these factors by 10 and later
 * dividing adjusted clock value by 10.
 */
-   if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
-   crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB)
+   if (output_format == INTEL_OUTPUT_FORMAT_YCBCR444 ||
+   output_format == INTEL_OUTPUT_FORMAT_RGB)
kslice_adjust = 10;
else
kslice_adjust = 5;
@@ -3103,7 +3105,7 @@ intel_hdmi_dsc_get_num_slices(const struct 
intel_crtc_state *crtc_state,
else
return 0;
 
-   slice_width = 
DIV_ROUND_UP(crtc_state->hw.adjusted_mode.hdisplay, target_slices);
+   slice_width = DIV_ROUND_UP(mode->hdisplay, target_slices);
if (slice_width >= max_slice_width)
min_slices = target_slices + 1;
} while (slice_width >= max_slice_width);
@@ -3119,6 +3121,7 @@ intel_hdmi_dsc_get_num_slices(const struct 
intel_crtc_state *crtc_state,
  * @slice_width: dsc slice width 

[Intel-gfx] [PATCH v2 1/4] drm/i915/hdmi: Fix the definition of intel_hdmi_dsc_get_bpp

2022-01-31 Thread Ankit Nautiyal
Fix the data-type of the argument output_format to enum, for the
function intel_hdmi_dsc_get_bpp.

v2: Fixed formatting issues in commit message (Jani).
Avoided including the header intel_display_types.h, instead used
forward declaration for the appropriate enum. (Jani).

Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC 
parameters for HDMI2.1")
Cc: Uma Shankar 
Cc: Jani Nikula 
Signed-off-by: Ankit Nautiyal 
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++--
 drivers/gpu/drm/i915/display/intel_hdmi.h | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 45cf0ab04009..381a9de3a015 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3126,8 +3126,8 @@ intel_hdmi_dsc_get_num_slices(const struct 
intel_crtc_state *crtc_state,
  */
 int
 intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
-  int output_format, bool hdmi_all_bpp,
-  int hdmi_max_chunk_bytes)
+  enum intel_output_format output_format,
+  bool hdmi_all_bpp, int hdmi_max_chunk_bytes)
 {
int max_dsc_bpp, min_dsc_bpp;
int target_bytes;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h 
b/drivers/gpu/drm/i915/display/intel_hdmi.h
index b577c38fa90c..ea2a3456bd4b 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.h
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
@@ -22,6 +22,7 @@ struct intel_hdmi;
 struct drm_connector_state;
 union hdmi_infoframe;
 enum port;
+enum intel_output_format;
 
 void intel_hdmi_init_connector(struct intel_digital_port *dig_port,
   struct intel_connector *intel_connector);
@@ -49,8 +50,8 @@ bool intel_hdmi_limited_color_range(const struct 
intel_crtc_state *crtc_state,
 bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
 int bpc, bool has_hdmi_sink, bool ycbcr420_output);
 int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
-  int num_slices, int output_format, bool hdmi_all_bpp,
-  int hdmi_max_chunk_bytes);
+  int num_slices, enum intel_output_format 
output_format,
+  bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
 int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
  int src_max_slices, int src_max_slice_width,
  int hdmi_max_slices, int hdmi_throughput);
-- 
2.25.1



[Intel-gfx] [PATCH v2 3/4] drm/i915/dp: Use the drm helpers for getting max FRL rate

2022-01-31 Thread Ankit Nautiyal
Re-use the drm helpers for getting max FRL rate for an HDMI sink.
This patch removes the duplicate code and calls the already defined
drm helpers for the task.

Signed-off-by: Ankit Nautiyal 
---
 drivers/gpu/drm/i915/display/intel_dp.c | 19 +--
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 4d4579a301f6..f7fe7de7e553 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2190,22 +2190,13 @@ static int intel_dp_hdmi_sink_max_frl(struct intel_dp 
*intel_dp)
 {
struct intel_connector *intel_connector = intel_dp->attached_connector;
struct drm_connector *connector = &intel_connector->base;
-   int max_frl_rate;
-   int max_lanes, rate_per_lane;
-   int max_dsc_lanes, dsc_rate_per_lane;
+   int max_frl = drm_hdmi_sink_max_frl(connector);
+   int dsc_max_frl = drm_hdmi_sink_dsc_max_frl(connector);
 
-   max_lanes = connector->display_info.hdmi.max_lanes;
-   rate_per_lane = connector->display_info.hdmi.max_frl_rate_per_lane;
-   max_frl_rate = max_lanes * rate_per_lane;
+   if (dsc_max_frl)
+   return min(max_frl, dsc_max_frl);
 
-   if (connector->display_info.hdmi.dsc_cap.v_1p2) {
-   max_dsc_lanes = connector->display_info.hdmi.dsc_cap.max_lanes;
-   dsc_rate_per_lane = 
connector->display_info.hdmi.dsc_cap.max_frl_rate_per_lane;
-   if (max_dsc_lanes && dsc_rate_per_lane)
-   max_frl_rate = min(max_frl_rate, max_dsc_lanes * 
dsc_rate_per_lane);
-   }
-
-   return max_frl_rate;
+   return max_frl;
 }
 
 static bool
-- 
2.25.1



[Intel-gfx] [PATCH v2 2/4] drm/edid: Add helper to get max FRL rate for an HDMI sink

2022-01-31 Thread Ankit Nautiyal
Add the helpers for getting the max FRL rate with and without DSC
for an HDMI sink.

v2: Fix the subject line and documentation of the helpers (Jani).
Split the helper definitions and usage into separate patches. (Jani).

Signed-off-by: Ankit Nautiyal 
---
 drivers/gpu/drm/drm_edid.c | 38 ++
 include/drm/drm_edid.h |  2 ++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index eb61a1a92dc0..c209fd6b24a2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -6176,3 +6176,41 @@ void drm_update_tile_info(struct drm_connector 
*connector,
connector->tile_group = NULL;
}
 }
+
+/**
+ * drm_hdmi_sink_max_frl - get the max frl rate, if supported
+ * @connector - connector with HDMI sink
+ *
+ * RETURNS:
+ * max frl rate supported by the HDMI sink, 0 if FRL not supported
+ */
+int drm_hdmi_sink_max_frl(struct drm_connector *connector)
+{
+   int max_lanes = connector->display_info.hdmi.max_lanes;
+   int rate_per_lane = connector->display_info.hdmi.max_frl_rate_per_lane;
+
+   return max_lanes * rate_per_lane;
+}
+EXPORT_SYMBOL(drm_hdmi_sink_max_frl);
+
+/**
+ * drm_hdmi_sink_dsc_max_frl - get the max frl rate from HDMI sink with
+ * DSC1.2 compression.
+ * @connector - connector with HDMI sink
+ *
+ * RETURNS:
+ * max frl rate supported by the HDMI sink with DSC1.2, 0 if FRL not supported
+ */
+int drm_hdmi_sink_dsc_max_frl(struct drm_connector *connector)
+{
+   int max_dsc_lanes, dsc_rate_per_lane;
+
+   if (!connector->display_info.hdmi.dsc_cap.v_1p2)
+   return 0;
+
+   max_dsc_lanes = connector->display_info.hdmi.dsc_cap.max_lanes;
+   dsc_rate_per_lane = 
connector->display_info.hdmi.dsc_cap.max_frl_rate_per_lane;
+
+   return max_dsc_lanes * dsc_rate_per_lane;
+}
+EXPORT_SYMBOL(drm_hdmi_sink_dsc_max_frl);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 18f6c700f6d0..5003e1254c44 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -592,6 +592,8 @@ drm_display_mode_from_cea_vic(struct drm_device *dev,
  u8 video_code);
 const u8 *drm_find_edid_extension(const struct edid *edid,
  int ext_id, int *ext_index);
+int drm_hdmi_sink_max_frl(struct drm_connector *connector);
+int drm_hdmi_sink_dsc_max_frl(struct drm_connector *connector);
 
 
 #endif /* __DRM_EDID_H__ */
-- 
2.25.1



[Intel-gfx] [PATCH v2 0/4] Minor Fixes and Refactoring for HDMI PCON stuff

2022-01-31 Thread Ankit Nautiyal
Misc fixes and refactoring in HDMI2.1 PCON helper functions.
V2:
Addressed review comments from Jani.
Splitted the drm_helper addition and usage in separate patches.

Ankit Nautiyal (4):
  drm/i915/hdmi: Fix the definition of intel_hdmi_dsc_get_bpp
  drm/edid: Add helper to get max FRL rate for an HDMI sink
  drm/i915/dp: Use the drm helpers for getting max FRL rate
  drm/i915/display: Simplify helpers for getting DSC slices and bpp

 drivers/gpu/drm/drm_edid.c| 38 +++
 drivers/gpu/drm/i915/display/intel_dp.c   | 26 ++--
 drivers/gpu/drm/i915/display/intel_hdmi.c | 26 +---
 drivers/gpu/drm/i915/display/intel_hdmi.h |  9 --
 include/drm/drm_edid.h|  2 ++
 5 files changed, 70 insertions(+), 31 deletions(-)

-- 
2.25.1



[Intel-gfx] [PATCH] drm/i915/ttm: Return some errors instead of trying memcpy move

2022-01-31 Thread Thomas Hellström
The i915_ttm_accel_move() function may return error codes that should
be propagated further up the stack rather than consumed assuming that
the accel move failed and could be replaced with a memcpy move.

For -EINTR, -ERESTARTSYS and -EAGAIN, just propagate those codes, rather
than retrying with a memcpy move.

Fixes: 2b0a750caf33 ("drm/i915/ttm: Failsafe migration blits")
Cc: Matthew Auld 
Signed-off-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 1de306c03aaf..1ebe6e4086a1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -436,11 +436,17 @@ __i915_ttm_move(struct ttm_buffer_object *bo,
 
if (!IS_ERR(fence))
goto out;
-   } else if (move_deps) {
-   int err = i915_deps_sync(move_deps, ctx);
+   } else {
+   int err = PTR_ERR(fence);
+
+   if (err == -EINTR || err == -ERESTARTSYS || err == -EAGAIN)
+   return fence;
 
-   if (err)
-   return ERR_PTR(err);
+   if (move_deps) {
+   err = i915_deps_sync(move_deps, ctx);
+   if (err)
+   return ERR_PTR(err);
+   }
}
 
/* Error intercept failed or no accelerated migration to start with */
-- 
2.34.1



Re: [Intel-gfx] [PATCH 1/3] drm: add writeback pointers to drm_connector

2022-01-31 Thread Kandpal, Suraj
Hey,
> I think there are more places affected with this change. I can get below
> compilation issues while trying to compile my branch:
> 
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘encoder_to_vc4_txp’:
> ./include/linux/build_bug.h:78:41: error: static assertion failed:
> "pointer type mismatch in container_of()"
>   #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>   ^
> ./include/linux/build_bug.h:77:34: note: in expansion of macro
> ‘__static_assert’
>   #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__,
> #expr)
>^
> ./include/linux/container_of.h:19:2: note: in expansion of macro
> ‘static_assert’
>static_assert(__same_type(*(ptr), ((type *)0)->member) || \
>^
> drivers/gpu/drm/vc4/vc4_txp.c:162:9: note: in expansion of macro
> ‘container_of’
>return container_of(encoder, struct vc4_txp, connector.encoder);
>   ^
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘connector_to_vc4_txp’:
> ./include/linux/build_bug.h:78:41: error: static assertion failed:
> "pointer type mismatch in container_of()"
>   #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>   ^
> ./include/linux/build_bug.h:77:34: note: in expansion of macro
> ‘__static_assert’
>   #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__,
> #expr)
>^
> ./include/linux/container_of.h:19:2: note: in expansion of macro
> ‘static_assert’
>static_assert(__same_type(*(ptr), ((type *)0)->member) || \
>^
> drivers/gpu/drm/vc4/vc4_txp.c:167:9: note: in expansion of macro
> ‘container_of’
>return container_of(conn, struct vc4_txp, connector.base);
>   ^
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_bind’:
> drivers/gpu/drm/vc4/vc4_txp.c:495:27: error: passing argument 1 of
> ‘drm_connector_helper_add’ from incompatible pointer type [-
> Werror=incompatible-pointer-types]
>drm_connector_helper_add(&txp->connector.base,
> ^
> In file included from ./include/drm/drm_atomic_helper.h:32:0,
>   from drivers/gpu/drm/vc4/vc4_txp.c:17:
> ./include/drm/drm_modeset_helper_vtables.h:1153:20: note: expected
> ‘struct drm_connector *’ but argument is of type ‘struct drm_connector **’
>   static inline void drm_connector_helper_add(struct drm_connector
> *connector,
>  ^
> drivers/gpu/drm/vc4/vc4_txp.c:509:10: error: assignment from incompatible
> pointer type [-Werror=incompatible-pointer-types]
>encoder = &txp->connector.encoder;
>^
> drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_unbind’:
> drivers/gpu/drm/vc4/vc4_txp.c:532:28: error: passing argument 1 of
> ‘vc4_txp_connector_destroy’ from incompatible pointer type [-
> Werror=incompatible-pointer-types]
>vc4_txp_connector_destroy(&txp->connector.base);
>  ^
> drivers/gpu/drm/vc4/vc4_txp.c:333:13: note: expected ‘struct
> drm_connector *’ but argument is of type ‘struct drm_connector **’
>   static void vc4_txp_connector_destroy(struct drm_connector *connector)
> 
I will have a look at this and try to resolve it
> 
> Looks like vc4 doesnt have its own encoder so we might have to move it to
> have its own?

Right seems like we will have to add a drm_connector and drm_encoder in the 
below structure
> 
> struct vc4_txp {
>  struct vc4_crtc base;
> 
>  struct platform_device *pdev;
> 
>  struct drm_writeback_connector connector;
> 
>  void __iomem *regs;
>  struct debugfs_regset32 regset;
> };
> 
> static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder
> *encoder)
> {
>  return container_of(encoder, struct vc4_txp, connector.encoder); }
> 
> static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector
> *conn)
> {
>  return container_of(conn, struct vc4_txp, connector.base); }
> 

Changes will be required here in both connector_of functions to point to 
The new drm_connector and drm_encoder we add in vc4_txp struct.

> 
> One more thing, it looks like to me, we need to do one more thing.
> 
> Like intel does and what MSM will do, the encoder pointer of the wb
> connector has to point to the encoder struct .
> 
>  >> wb_conn = &intel_connector->wb_conn;
>  >> wb_conn->base = &intel_connector->base;
>  >> wb_conn->encoder = &intel_wd->base.base;
> 
Yes this will need to be done 

Thanks Abhinav for pointing Ill implement this and send it in the next version
of patches 

Regards,
Suraj Kandpal



Re: [Intel-gfx] [PATCH 1/3] drm: add writeback pointers to drm_connector

2022-01-31 Thread Abhinav Kumar

Hi Suraj

I think there are more places affected with this change. I can get below 
compilation issues while trying to compile my branch:


drivers/gpu/drm/vc4/vc4_txp.c: In function ‘encoder_to_vc4_txp’:
./include/linux/build_bug.h:78:41: error: static assertion failed: 
"pointer type mismatch in container_of()"

 #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
 ^
./include/linux/build_bug.h:77:34: note: in expansion of macro 
‘__static_assert’
 #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, 
#expr)

  ^
./include/linux/container_of.h:19:2: note: in expansion of macro 
‘static_assert’

  static_assert(__same_type(*(ptr), ((type *)0)->member) || \
  ^
drivers/gpu/drm/vc4/vc4_txp.c:162:9: note: in expansion of macro 
‘container_of’

  return container_of(encoder, struct vc4_txp, connector.encoder);
 ^
drivers/gpu/drm/vc4/vc4_txp.c: In function ‘connector_to_vc4_txp’:
./include/linux/build_bug.h:78:41: error: static assertion failed: 
"pointer type mismatch in container_of()"

 #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
 ^
./include/linux/build_bug.h:77:34: note: in expansion of macro 
‘__static_assert’
 #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, 
#expr)

  ^
./include/linux/container_of.h:19:2: note: in expansion of macro 
‘static_assert’

  static_assert(__same_type(*(ptr), ((type *)0)->member) || \
  ^
drivers/gpu/drm/vc4/vc4_txp.c:167:9: note: in expansion of macro 
‘container_of’

  return container_of(conn, struct vc4_txp, connector.base);
 ^
drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_bind’:
drivers/gpu/drm/vc4/vc4_txp.c:495:27: error: passing argument 1 of 
‘drm_connector_helper_add’ from incompatible pointer type 
[-Werror=incompatible-pointer-types]

  drm_connector_helper_add(&txp->connector.base,
   ^
In file included from ./include/drm/drm_atomic_helper.h:32:0,
 from drivers/gpu/drm/vc4/vc4_txp.c:17:
./include/drm/drm_modeset_helper_vtables.h:1153:20: note: expected 
‘struct drm_connector *’ but argument is of type ‘struct drm_connector **’
 static inline void drm_connector_helper_add(struct drm_connector 
*connector,

^
drivers/gpu/drm/vc4/vc4_txp.c:509:10: error: assignment from 
incompatible pointer type [-Werror=incompatible-pointer-types]

  encoder = &txp->connector.encoder;
  ^
drivers/gpu/drm/vc4/vc4_txp.c: In function ‘vc4_txp_unbind’:
drivers/gpu/drm/vc4/vc4_txp.c:532:28: error: passing argument 1 of 
‘vc4_txp_connector_destroy’ from incompatible pointer type 
[-Werror=incompatible-pointer-types]

  vc4_txp_connector_destroy(&txp->connector.base);
^
drivers/gpu/drm/vc4/vc4_txp.c:333:13: note: expected ‘struct 
drm_connector *’ but argument is of type ‘struct drm_connector **’

 static void vc4_txp_connector_destroy(struct drm_connector *connector)


Looks like vc4 doesnt have its own encoder so we might have to move it
to have its own?

struct vc4_txp {
struct vc4_crtc base;

struct platform_device *pdev;

struct drm_writeback_connector connector;

void __iomem *regs;
struct debugfs_regset32 regset;
};

static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder 
*encoder)

{
return container_of(encoder, struct vc4_txp, connector.encoder);
}

static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector 
*conn)

{
return container_of(conn, struct vc4_txp, connector.base);
}


One more thing, it looks like to me, we need to do one more thing.

Like intel does and what MSM will do, the encoder pointer of the wb 
connector has to point to the encoder struct .


>> wb_conn = &intel_connector->wb_conn;
>> wb_conn->base = &intel_connector->base;
>> wb_conn->encoder = &intel_wd->base.base;

Thanks

Abhinav
On 1/27/2022 10:17 AM, Abhinav Kumar wrote:

Hi Suraj

Thanks for the response.

I was not too worried about the intel driver as I am sure you must have 
validated this change with that :)


My question was more for the other vendor writeback drivers.

Thanks for looking into the others and providing the snippets.
After looking at those, yes I also think it should work.

So, if others do not have any concern with this change,

Reviewed-by: Abhinav Kumar 

On 1/27/2022 1:33 AM, Kandpal, Suraj wrote:


+ laurent on this

Hi Suraj
Jani pointed me to this thread as i had posted something similar here :
https://patchwork.freedesktop.org/patch/470296/ but since this thread 
was

posted earlier, we can discuss further here.

Overall, its similar to what I had posted in the RFC and your commit 
text also

covers my concerns too.

One question I have about your change is since you have changed
wb_connector::encoder to be a pointer, i saw the other changes in the 
series
but they do not allocate an encoder

Re: [Intel-gfx] [PATCH v2] drm/i915/display/vrr: Reset VRR capable property on a long hpd

2022-01-31 Thread Navare, Manasi
On Mon, Jan 31, 2022 at 02:28:04PM +0200, Jani Nikula wrote:
> On Wed, 26 Jan 2022, Manasi Navare  wrote:
> > With some VRR panels, user can turn VRR ON/OFF on the fly from the panel 
> > settings.
> > When VRR is turned OFF ,sends a long HPD to the driver clearing the Ignore 
> > MSA bit
> > in the DPCD. Currently the driver parses that onevery HPD but fails to reset
> > the corresponding VRR Capable Connector property.
> > Hence the userspace still sees this as VRR Capable panel which is incorrect.
> >
> > Fix this by explicitly resetting the connector property.
> >
> > v2: Reset vrr capable if status == connector_disconnected
> > v3: Use i915 and use bool vrr_capable (Jani Nikula)
> > Cc: Jani Nikula 
> > Signed-off-by: Manasi Navare 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 16 
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 4d4579a301f6..687cb37bb22a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4446,6 +4446,12 @@ intel_dp_detect(struct drm_connector *connector,
> > memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> > memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> >  
> > +   /* Reset VRR Capable property */
> > +   drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s] VRR capable: 
> > FALSE\n",
> > +   connector->base.id, connector->name);
> 
> Both the comment and the logging here seem excessive.

Okay will remove the comment, it is indeed redundant now
> 
> > +   drm_connector_set_vrr_capable_property(connector,
> > +  false);
> > +
> 
> Fits on one line.

Sure will change that
> 
> > if (intel_dp->is_mst) {
> > drm_dbg_kms(&dev_priv->drm,
> > "MST device may have disappeared %d vs 
> > %d\n",
> > @@ -4560,15 +4566,17 @@ static int intel_dp_get_modes(struct drm_connector 
> > *connector)
> >  {
> > struct intel_connector *intel_connector = to_intel_connector(connector);
> > struct edid *edid;
> > +   struct drm_i915_private *i915 = to_i915(connector->dev);
> > int num_modes = 0;
> >  
> > edid = intel_connector->detect_edid;
> > if (edid) {
> > -   num_modes = intel_connector_update_modes(connector, edid);
> > +   bool vrr_capable = intel_vrr_is_capable(connector);
> >  
> > -   if (intel_vrr_is_capable(connector))
> > -   drm_connector_set_vrr_capable_property(connector,
> > -  true);
> > +   num_modes = intel_connector_update_modes(connector, edid);
> 
> intel_vrr_is_capable() probably needs to happen after
> intel_connector_update_modes(), right?

Thanks Jani yes that is correct, will move this after num_modes = 
intel_connector_update_modes(connector, edid);

Manasi

> 
> BR,
> Jani.
> 
> > +   drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> > +   connector->base.id, connector->name, 
> > yesno(vrr_capable));
> > +   drm_connector_set_vrr_capable_property(connector, vrr_capable);
> > }
> >  
> > /* Also add fixed mode, which may or may not be present in EDID */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH 13/21] fbcon: move more common code into fb_open()

2022-01-31 Thread kernel test robot
Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tegra-drm/drm/tegra/for-next]
[also build test WARNING on drm/drm-next linus/master v5.17-rc2 next-20220131]
[cannot apply to airlied/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907
base:   git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next
config: mips-allyesconfig 
(https://download.01.org/0day-ci/archive/20220201/202202010613.ht19hztx-...@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/0day-ci/linux/commit/7a27c0ce71acfa8b67787d298de9836376fcc323
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Daniel-Vetter/some-fbcon-patches-mostly-locking/20220201-050907
git checkout 7a27c0ce71acfa8b67787d298de9836376fcc323
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross 
O=build_dir ARCH=mips SHELL=/bin/bash drivers/video/fbdev/core/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

   drivers/video/fbdev/core/fbcon.c: In function 'con2fb_acquire_newinfo':
>> drivers/video/fbdev/core/fbcon.c:721:27: warning: variable 'ops' set but not 
>> used [-Wunused-but-set-variable]
 721 | struct fbcon_ops *ops = NULL;
 |   ^~~


vim +/ops +721 drivers/video/fbdev/core/fbcon.c

^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
717  
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
718  static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
7a27c0ce71acfa8 drivers/video/fbdev/core/fbcon.c Daniel Vetter  2022-01-31  
719   int unit)
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
720  {
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 
@721 struct fbcon_ops *ops = NULL;
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter  2022-01-31  
722 int err;
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
723  
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter  2022-01-31  
724 err = fbcon_open(info);
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter  2022-01-31  
725 if (err)
ba35a78b17408b6 drivers/video/fbdev/core/fbcon.c Daniel Vetter  2022-01-31  
726 return err;
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
727  
7a27c0ce71acfa8 drivers/video/fbdev/core/fbcon.c Daniel Vetter  2022-01-31  
728 ops = info->fbcon_par;
d1baa4ffa677bf6 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17  
729  
d1baa4ffa677bf6 drivers/video/console/fbcon.cAntonino A. Daplas 2007-07-17  
730 if (vc)
b73deed32d08740 drivers/video/console/fbcon.cAntonino A. Daplas 2006-01-09  
731 set_blitting_type(vc, info);
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
732  
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
733 return err;
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
734  }
^1da177e4c3f415 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16  
735  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


[Intel-gfx] [drm-intel:for-linux-next-fixes 6/7] drivers/gpu/drm/i915/i915_vma.c:451:4: error: 'ret' undeclared; did you mean 'net'?

2022-01-31 Thread kernel test robot
tree:   git://anongit.freedesktop.org/drm-intel for-linux-next-fixes
head:   ea33c6d63f87e34b14dba6f2804990a5fc5a60d7
commit: 2e872d87cbf2cd02dca570ee187cf35567576a70 [6/7] drm/i915: delete shadow 
"ret" variable
config: x86_64-randconfig-a003-20220131 
(https://download.01.org/0day-ci/archive/20220201/202202010616.uhgpmfp0-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
git remote add drm-intel git://anongit.freedesktop.org/drm-intel
git fetch --no-tags drm-intel for-linux-next-fixes
git checkout 2e872d87cbf2cd02dca570ee187cf35567576a70
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind':
>> drivers/gpu/drm/i915/i915_vma.c:451:4: error: 'ret' undeclared (first use in 
>> this function); did you mean 'net'?
 451 |ret = i915_gem_object_wait_moving_fence(vma->obj, true);
 |^~~
 |net
   drivers/gpu/drm/i915/i915_vma.c:451:4: note: each undeclared identifier is 
reported only once for each function it appears in


vim +451 drivers/gpu/drm/i915/i915_vma.c

f6c466b84cfa78 Maarten Lankhorst  2021-11-22  376  
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  377  /**
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  378   * i915_vma_bind - Sets 
up PTEs for an VMA in it's corresponding address space.
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  379   * @vma: VMA to map
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  380   * @cache_level: mapping 
cache level
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  381   * @flags: flags like 
global or local mapping
2850748ef8763a Chris Wilson   2019-10-04  382   * @work: preallocated 
worker for allocating and binding the PTE
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  383   *
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  384   * DMA addresses are 
taken from the scatter-gather table of this object (or of
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  385   * this VMA in case of 
non-default GGTT views) and PTE entries set up.
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  386   * Note that DMA 
addresses are also the only part of the SG table we care about.
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  387   */
2850748ef8763a Chris Wilson   2019-10-04  388  int i915_vma_bind(struct 
i915_vma *vma,
2850748ef8763a Chris Wilson   2019-10-04  389 enum 
i915_cache_level cache_level,
2850748ef8763a Chris Wilson   2019-10-04  390 u32 flags,
2850748ef8763a Chris Wilson   2019-10-04  391 struct 
i915_vma_work *work)
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  392  {
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  393   u32 bind_flags;
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  394   u32 vma_flags;
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  395  
c2ea703dcafccf Thomas Hellström   2021-12-21  396   
lockdep_assert_held(&vma->vm->mutex);
aa149431279166 Chris Wilson   2017-02-25  397   
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
aa149431279166 Chris Wilson   2017-02-25  398   GEM_BUG_ON(vma->size > 
vma->node.size);
aa149431279166 Chris Wilson   2017-02-25  399  
bbb8a9d7e000c9 Tvrtko Ursulin 2018-10-12  400   if 
(GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
aa149431279166 Chris Wilson   2017-02-25  401   
  vma->node.size,
aa149431279166 Chris Wilson   2017-02-25  402   
  vma->vm->total)))
aa149431279166 Chris Wilson   2017-02-25  403   return -ENODEV;
aa149431279166 Chris Wilson   2017-02-25  404  
bbb8a9d7e000c9 Tvrtko Ursulin 2018-10-12  405   if 
(GEM_DEBUG_WARN_ON(!flags))
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  406   return -EINVAL;
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  407  
2850748ef8763a Chris Wilson   2019-10-04  408   bind_flags = flags;
2850748ef8763a Chris Wilson   2019-10-04  409   bind_flags &= 
I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  410  
4dd2fbbfb532d0 Chris Wilson   2019-09-11  411   vma_flags = 
atomic_read(&vma->flags);
4dd2fbbfb532d0 Chris Wilson   2019-09-11  412   vma_flags &= 
I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
aedbe0a1af585e Chris Wilson   2020-05-21  413  
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  414   bind_flags &= 
~vma_flags;
b42fe9ca0a1e2b Joonas Lahtinen2016-11-11  415   if (bind_flags == 0)

[Intel-gfx] linux-next: build failure after merge of the drm-intel-fixes tree

2022-01-31 Thread Stephen Rothwell
Hi all,

After merging the drm-intel-fixes tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind':
drivers/gpu/drm/i915/i915_vma.c:451:25: error: 'ret' undeclared (first use in 
this function); did you mean 'net'?
  451 | ret = 
i915_gem_object_wait_moving_fence(vma->obj, true);
  | ^~~
  | net

Caused by commit

  2e872d87cbf2 ("drm/i915: delete shadow "ret" variable")

I have reverted that commit for today.

-- 
Cheers,
Stephen Rothwell


pgpY_DgIRsF_x.pgp
Description: OpenPGP digital signature


[Intel-gfx] ✗ Fi.CI.BAT: failure for some fbcon patches, mostly locking

2022-01-31 Thread Patchwork
== Series Details ==

Series: some fbcon patches, mostly locking
URL   : https://patchwork.freedesktop.org/series/99549/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11166 -> Patchwork_22144


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_22144 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22144, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/index.html

Participating hosts (46 -> 44)
--

  Additional (1): fi-pnv-d510 
  Missing(3): fi-jsl-1 shard-tglu fi-bdw-samus 

Possible new issues
---

  Here are the unknown changes that may have been introduced in Patchwork_22144:

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@hangcheck:
- fi-bdw-5557u:   NOTRUN -> [INCOMPLETE][1]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-bdw-5557u/igt@i915_selftest@l...@hangcheck.html

  * igt@i915_selftest@live@reset:
- bat-adlp-4: [PASS][2] -> [DMESG-FAIL][3]
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/bat-adlp-4/igt@i915_selftest@l...@reset.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/bat-adlp-4/igt@i915_selftest@l...@reset.html

  
 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@workarounds:
- {bat-adlp-6}:   [PASS][4] -> [DMESG-WARN][5]
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/bat-adlp-6/igt@i915_selftest@l...@workarounds.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/bat-adlp-6/igt@i915_selftest@l...@workarounds.html

  
Known issues


  Here are the changes found in Patchwork_22144 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@amdgpu/amd_cs_nop@fork-compute0:
- fi-blb-e6850:   NOTRUN -> [SKIP][6] ([fdo#109271]) +17 similar issues
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-blb-e6850/igt@amdgpu/amd_cs_...@fork-compute0.html

  * igt@gem_huc_copy@huc-copy:
- fi-skl-6600u:   NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#2190])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-skl-6600u/igt@gem_huc_c...@huc-copy.html
- fi-pnv-d510:NOTRUN -> [SKIP][8] ([fdo#109271]) +57 similar issues
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-pnv-d510/igt@gem_huc_c...@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
- fi-skl-6600u:   NOTRUN -> [SKIP][9] ([fdo#109271] / [i915#4613]) +3 
similar issues
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-skl-6600u/igt@gem_lmem_swapp...@verify-random.html

  * igt@i915_selftest@live@hangcheck:
- fi-hsw-4770:[PASS][10] -> [INCOMPLETE][11] ([i915#4785])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html

  * igt@kms_chamelium@vga-edid-read:
- fi-skl-6600u:   NOTRUN -> [SKIP][12] ([fdo#109271] / [fdo#111827]) +8 
similar issues
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-skl-6600u/igt@kms_chamel...@vga-edid-read.html
- fi-bdw-5557u:   NOTRUN -> [SKIP][13] ([fdo#109271] / [fdo#111827]) +8 
similar issues
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-bdw-5557u/igt@kms_chamel...@vga-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-skl-6600u:   NOTRUN -> [SKIP][14] ([fdo#109271]) +2 similar issues
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-skl-6600u/igt@kms_cursor_leg...@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_frontbuffer_tracking@basic:
- fi-cml-u2:  [PASS][15] -> [DMESG-WARN][16] ([i915#4269])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-cml-u2/igt@kms_frontbuffer_track...@basic.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-cml-u2/igt@kms_frontbuffer_track...@basic.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-skl-6600u:   NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#533])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-skl-6600u/igt@kms_pipe_crc_ba...@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_psr@cursor_plane_move:
- fi-bdw-5557u:   NOTRUN -> [SKIP][18] ([fdo#109271]) +13 similar issues
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22144/fi-bdw-5557u/igt@kms_psr@cursor_p

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for some fbcon patches, mostly locking

2022-01-31 Thread Patchwork
== Series Details ==

Series: some fbcon patches, mostly locking
URL   : https://patchwork.freedesktop.org/series/99549/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for some fbcon patches, mostly locking

2022-01-31 Thread Patchwork
== Series Details ==

Series: some fbcon patches, mostly locking
URL   : https://patchwork.freedesktop.org/series/99549/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
8895b11855ce MAINTAINERS: Add entry for fbdev core
-:64: WARNING:MAINTAINERS_STYLE: Misordered MAINTAINERS entry - list 'S:' 
before 'F:'
#64: FILE: MAINTAINERS:7582:
+F: drivers/video/fbdev/core/
+S: Odd Fixes

-:69: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address 
mismatch: 'From: Daniel Vetter ' != 'Signed-off-by: 
Daniel Vetter '

total: 0 errors, 2 warnings, 0 checks, 12 lines checked
9029711bda24 fbcon: Resurrect fbcon accelerated scrolling code
-:24: WARNING:REPEATED_WORD: Possible repeated word: 'warnings'
#24: 
And finally to shut up unused parameter warnings warnings the macros

-:27: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description 
(prefer a maximum 75 chars per line)
#27: 
References: 
https://lore.kernel.org/dri-devel/feea8303-2b83-fc36-972c-4fc8ad723...@gmx.de/

-:41: WARNING:BAD_SIGN_OFF: Duplicate signature
#41: 
Cc: Claudio Suarez 

-:209: WARNING:INLINE: plain inline is preferred over __inline__
#209: FILE: drivers/video/fbdev/core/fbcon.c:1437:
+static __inline__ void ywrap_up(struct vc_data *vc, int count)

-:228: WARNING:INLINE: plain inline is preferred over __inline__
#228: FILE: drivers/video/fbdev/core/fbcon.c:1456:
+static __inline__ void ywrap_down(struct vc_data *vc, int count)

-:247: WARNING:INLINE: plain inline is preferred over __inline__
#247: FILE: drivers/video/fbdev/core/fbcon.c:1475:
+static __inline__ void ypan_up(struct vc_data *vc, int count)

-:271: WARNING:INLINE: plain inline is preferred over __inline__
#271: FILE: drivers/video/fbdev/core/fbcon.c:1499:
+static __inline__ void ypan_up_redraw(struct vc_data *vc, int t, int count)

-:295: WARNING:INLINE: plain inline is preferred over __inline__
#295: FILE: drivers/video/fbdev/core/fbcon.c:1523:
+static __inline__ void ypan_down(struct vc_data *vc, int count)

-:319: WARNING:INLINE: plain inline is preferred over __inline__
#319: FILE: drivers/video/fbdev/core/fbcon.c:1547:
+static __inline__ void ypan_down_redraw(struct vc_data *vc, int t, int count)

-:409: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#409: FILE: drivers/video/fbdev/core/fbcon.c:1637:
+static void fbcon_redraw_blit(struct vc_data *vc, struct fb_info *info,
+   struct fbcon_display *p, int line, int count, int 
ycount)

-:429: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#429: FILE: drivers/video/fbdev/core/fbcon.c:1657:
+  line, x, 1, s-start);
^

-:445: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#445: FILE: drivers/video/fbdev/core/fbcon.c:1673:
+  s-start);
^

-:449: CHECK:BRACES: Unbalanced braces around else statement
#449: FILE: drivers/video/fbdev/core/fbcon.c:1677:
+   else {

-:490: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#490: FILE: drivers/video/fbdev/core/fbcon.c:1782:
+   fbcon_redraw_blit(vc, info, p, t, b - t - count,
+count);

-:492: CHECK:SPACING: No space is necessary after a cast
#492: FILE: drivers/video/fbdev/core/fbcon.c:1784:
+   scr_memsetw((unsigned short *) (vc->vc_origin +

-:500: CHECK:BRACES: braces {} should be used on all arms of this statement
#500: FILE: drivers/video/fbdev/core/fbcon.c:1792:
+   if (b - t - count > 3 * vc->vc_rows >> 2) {
[...]
+   } else if (info->flags & FBINFO_READS_FAST)
[...]
+   else
[...]

-:518: CHECK:BRACES: braces {} should be used on all arms of this statement
#518: FILE: drivers/video/fbdev/core/fbcon.c:1810:
+   if ((p->yscroll + count <=
[...]
+   } else
[...]

-:520: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the 
previous line
#520: FILE: drivers/video/fbdev/core/fbcon.c:1812:
+2 * (p->vrows - vc->vc_rows))
+   && ((!scroll_partial && (b - t == vc->vc_rows))

-:521: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the 
previous line
#521: FILE: drivers/video/fbdev/core/fbcon.c:1813:
+   && ((!scroll_partial && (b - t == vc->vc_rows))
+   || (scroll_partial

-:522: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the 
previous line
#522: FILE: drivers/video/fbdev/core/fbcon.c:1814:
+   || (scroll_partial
+   && (b - t - count >

-:530: CHECK:BRACES: Unbalanced braces around else statement
#530: FILE: drivers/video/fbdev/core/fbcon.c:1822:
+   } else

-:536: CHECK:B

[Intel-gfx] ✗ Fi.CI.BAT: failure for discrete card 64K page support (rev4)

2022-01-31 Thread Patchwork
== Series Details ==

Series: discrete card 64K page support (rev4)
URL   : https://patchwork.freedesktop.org/series/99119/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11166 -> Patchwork_22143


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_22143 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22143, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/index.html

Participating hosts (45 -> 44)
--

  Additional (1): fi-pnv-d510 
  Missing(2): fi-icl-u2 fi-bdw-samus 

Possible new issues
---

  Here are the unknown changes that may have been introduced in Patchwork_22143:

### IGT changes ###

 Possible regressions 

  * igt@i915_selftest@live@gtt:
- fi-bsw-kefka:   [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-bsw-kefka/igt@i915_selftest@l...@gtt.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-bsw-kefka/igt@i915_selftest@l...@gtt.html
- fi-bwr-2160:[PASS][3] -> [DMESG-FAIL][4]
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-bwr-2160/igt@i915_selftest@l...@gtt.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-bwr-2160/igt@i915_selftest@l...@gtt.html
- fi-skl-guc: [PASS][5] -> [DMESG-FAIL][6]
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-skl-guc/igt@i915_selftest@l...@gtt.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-skl-guc/igt@i915_selftest@l...@gtt.html
- fi-kbl-8809g:   [PASS][7] -> [DMESG-FAIL][8]
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-kbl-8809g/igt@i915_selftest@l...@gtt.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-kbl-8809g/igt@i915_selftest@l...@gtt.html
- fi-blb-e6850:   NOTRUN -> [DMESG-FAIL][9]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-blb-e6850/igt@i915_selftest@l...@gtt.html
- fi-kbl-7567u:   [PASS][10] -> [DMESG-FAIL][11]
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-kbl-7567u/igt@i915_selftest@l...@gtt.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-kbl-7567u/igt@i915_selftest@l...@gtt.html
- bat-dg1-5:  [PASS][12] -> [DMESG-FAIL][13]
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/bat-dg1-5/igt@i915_selftest@l...@gtt.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/bat-dg1-5/igt@i915_selftest@l...@gtt.html
- fi-glk-j4005:   [PASS][14] -> [DMESG-FAIL][15]
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-glk-j4005/igt@i915_selftest@l...@gtt.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-glk-j4005/igt@i915_selftest@l...@gtt.html
- fi-bsw-nick:[PASS][16] -> [INCOMPLETE][17]
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-bsw-nick/igt@i915_selftest@l...@gtt.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-bsw-nick/igt@i915_selftest@l...@gtt.html
- fi-cfl-8109u:   [PASS][18] -> [DMESG-FAIL][19]
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-cfl-8109u/igt@i915_selftest@l...@gtt.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-cfl-8109u/igt@i915_selftest@l...@gtt.html
- fi-snb-2520m:   [PASS][20] -> [DMESG-FAIL][21]
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-snb-2520m/igt@i915_selftest@l...@gtt.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-snb-2520m/igt@i915_selftest@l...@gtt.html
- fi-cfl-8700k:   [PASS][22] -> [DMESG-FAIL][23]
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-cfl-8700k/igt@i915_selftest@l...@gtt.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-cfl-8700k/igt@i915_selftest@l...@gtt.html
- fi-bxt-dsi: [PASS][24] -> [DMESG-FAIL][25]
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-bxt-dsi/igt@i915_selftest@l...@gtt.html
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-bxt-dsi/igt@i915_selftest@l...@gtt.html
- fi-cml-u2:  [PASS][26] -> [DMESG-FAIL][27]
   [26]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-cml-u2/igt@i915_selftest@l...@gtt.html
   [27]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22143/fi-cml-u2/igt@i915_selftest@l...@gtt.html
- fi-ilk-650: [PASS][28] -> [DMESG-FAIL][29]
   [28]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11166/fi-ilk-650/igt@i915_selftest@l...@gtt.html
   [29]: 
https://intel-gfx-ci.01.org/tre

[Intel-gfx] [PATCH 21/21] fbdev: Make registered_fb[] private to fbmem.c

2022-01-31 Thread Daniel Vetter
Well except when the olpc dcon fbdev driver is enabled, that thing
digs around in there in rather unfixable ways.

Cc oldc_dcon maintainers as fyi.

Cc: Jens Frederich 
Cc: Jon Nettleton 
Cc: Greg Kroah-Hartman 
Cc: linux-stag...@lists.linux.dev
Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Helge Deller 
Cc: Matthew Wilcox 
Cc: Sam Ravnborg 
Cc: Tetsuo Handa 
Cc: Zhen Lei 
Cc: Alex Deucher 
Cc: Xiyu Yang 
Cc: linux-fb...@vger.kernel.org
Cc: Zheyu Ma 
Cc: Guenter Roeck 
---
 drivers/video/fbdev/core/fbmem.c | 8 ++--
 include/linux/fb.h   | 7 +++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 904ef1250677..dad6572942fa 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -48,10 +48,14 @@
 static DEFINE_MUTEX(registration_lock);
 
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
-EXPORT_SYMBOL(registered_fb);
-
 int num_registered_fb __read_mostly;
+#if IS_ENABLED(CONFIG_OLPC_DCON)
+EXPORT_SYMBOL(registered_fb);
 EXPORT_SYMBOL(num_registered_fb);
+#endif
+#define for_each_registered_fb(i)  \
+   for (i = 0; i < FB_MAX; i++)\
+   if (!registered_fb[i]) {} else
 
 bool fb_center_logo __read_mostly;
 
diff --git a/include/linux/fb.h b/include/linux/fb.h
index a8a00d2ba1f3..e236817502c2 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -622,16 +622,15 @@ extern int fb_get_color_depth(struct fb_var_screeninfo 
*var,
 extern int fb_get_options(const char *name, char **option);
 extern int fb_new_modelist(struct fb_info *info);
 
+#if IS_ENABLED(CONFIG_OLPC_DCON)
 extern struct fb_info *registered_fb[FB_MAX];
+
 extern int num_registered_fb;
+#endif
 extern bool fb_center_logo;
 extern int fb_logo_count;
 extern struct class *fb_class;
 
-#define for_each_registered_fb(i)  \
-   for (i = 0; i < FB_MAX; i++)\
-   if (!registered_fb[i]) {} else
-
 static inline void lock_fb_info(struct fb_info *info)
 {
mutex_lock(&info->lock);
-- 
2.33.0



[Intel-gfx] [PATCH 17/21] fbcon: Move more code into fbcon_release

2022-01-31 Thread Daniel Vetter
con2fb_release_oldinfo() has a bunch more kfree() calls than
fbcon_exit(), but since kfree() on NULL is harmless doing that in both
places should be ok. This is also a bit more symmetric now again with
fbcon_open also allocating the fbcon_ops structure.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Tetsuo Handa 
Cc: Greg Kroah-Hartman 
Cc: Du Cheng 
Cc: Claudio Suarez 
---
 drivers/video/fbdev/core/fbcon.c | 33 +---
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index e5e8aaf6f60d..5c14e24d14a1 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -688,6 +688,18 @@ static void fbcon_release(struct fb_info *info)
unlock_fb_info(info);
 
module_put(info->fbops->owner);
+
+   if (info->fbcon_par) {
+   struct fbcon_ops *ops = info->fbcon_par;
+
+   fbcon_del_cursor_work(info);
+   kfree(ops->cursor_state.mask);
+   kfree(ops->cursor_data);
+   kfree(ops->cursor_src);
+   kfree(ops->fontbuffer);
+   kfree(info->fbcon_par);
+   info->fbcon_par = NULL;
+   }
 }
 
 static int fbcon_open(struct fb_info *info)
@@ -741,18 +753,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, 
struct fb_info *info,
 static void con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
   struct fb_info *newinfo)
 {
-   struct fbcon_ops *ops = oldinfo->fbcon_par;
int ret;
 
fbcon_release(oldinfo);
 
-   fbcon_del_cursor_work(oldinfo);
-   kfree(ops->cursor_state.mask);
-   kfree(ops->cursor_data);
-   kfree(ops->cursor_src);
-   kfree(ops->fontbuffer);
-   kfree(oldinfo->fbcon_par);
-   oldinfo->fbcon_par = NULL;
/*
  If oldinfo and newinfo are driving the same hardware,
  the fb_release() method of oldinfo may attempt to
@@ -3335,19 +3339,8 @@ static void fbcon_exit(void)
}
}
 
-   if (mapped) {
-   if (info->fbcon_par) {
-   struct fbcon_ops *ops = info->fbcon_par;
-
-   fbcon_del_cursor_work(info);
-   kfree(ops->cursor_src);
-   kfree(ops->cursor_state.mask);
-   kfree(info->fbcon_par);
-   info->fbcon_par = NULL;
-   }
-
+   if (mapped)
fbcon_release(info);
-   }
}
 }
 
-- 
2.33.0



[Intel-gfx] [PATCH 20/21] Revert "fbdev: Prevent probing generic drivers if a FB is already registered"

2022-01-31 Thread Daniel Vetter
This reverts commit fb561bf9abde49f7e00fdbf9ed2ccf2d86cac8ee.

With

commit 27599aacbaefcbf2af7b06b0029459bbf682000d
Author: Thomas Zimmermann 
Date:   Tue Jan 25 10:12:18 2022 +0100

fbdev: Hot-unplug firmware fb devices on forced removal

this should be fixed properly and we can remove this somewhat hackish
check here (e.g. this won't catch drm drivers if fbdev emulation isn't
enabled).

Cc: Thomas Zimmermann 
Cc: Zack Rusin 
Cc: Javier Martinez Canillas 
Cc: Zack Rusin 
Cc: Hans de Goede 
Cc: Ilya Trukhanov 
Signed-off-by: Daniel Vetter 
Cc: Peter Jones 
Cc: linux-fb...@vger.kernel.org
---
 drivers/video/fbdev/efifb.c| 11 ---
 drivers/video/fbdev/simplefb.c | 11 ---
 2 files changed, 22 deletions(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index ea42ba6445b2..edca3703b964 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -351,17 +351,6 @@ static int efifb_probe(struct platform_device *dev)
char *option = NULL;
efi_memory_desc_t md;
 
-   /*
-* Generic drivers must not be registered if a framebuffer exists.
-* If a native driver was probed, the display hardware was already
-* taken and attempting to use the system framebuffer is dangerous.
-*/
-   if (num_registered_fb > 0) {
-   dev_err(&dev->dev,
-   "efifb: a framebuffer is already registered\n");
-   return -EINVAL;
-   }
-
if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
return -ENODEV;
 
diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 57541887188b..ee8b4412f7e4 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -407,17 +407,6 @@ static int simplefb_probe(struct platform_device *pdev)
struct simplefb_par *par;
struct resource *mem;
 
-   /*
-* Generic drivers must not be registered if a framebuffer exists.
-* If a native driver was probed, the display hardware was already
-* taken and attempting to use the system framebuffer is dangerous.
-*/
-   if (num_registered_fb > 0) {
-   dev_err(&pdev->dev,
-   "simplefb: a framebuffer is already registered\n");
-   return -EINVAL;
-   }
-
if (fb_get_options("simplefb", NULL))
return -ENODEV;
 
-- 
2.33.0



[Intel-gfx] [PATCH 15/21] fbcon: Consistently protect deferred_takeover with console_lock()

2022-01-31 Thread Daniel Vetter
This shouldn't be a problem in practice since until we've actually
taken over the console there's nothing we've registered with the
console/vt subsystem, so the exit/unbind path that check this can't
do the wrong thing. But it's confusing, so fix it by moving it a tad
later.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Du Cheng 
Cc: Tetsuo Handa 
Cc: Claudio Suarez 
Cc: Thomas Zimmermann 
---
 drivers/video/fbdev/core/fbcon.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 496bc5f2133e..11b9f962af6f 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -3247,6 +3247,9 @@ static void fbcon_register_existing_fbs(struct 
work_struct *work)
 
console_lock();
 
+   deferred_takeover = false;
+   logo_shown = FBCON_LOGO_DONTSHOW;
+
for_each_registered_fb(i)
fbcon_fb_registered(registered_fb[i]);
 
@@ -3264,8 +3267,6 @@ static int fbcon_output_notifier(struct notifier_block 
*nb,
pr_info("fbcon: Taking over console\n");
 
dummycon_unregister_output_notifier(&fbcon_output_nb);
-   deferred_takeover = false;
-   logo_shown = FBCON_LOGO_DONTSHOW;
 
/* We may get called in atomic context */
schedule_work(&fbcon_deferred_takeover_work);
-- 
2.33.0



[Intel-gfx] [PATCH 18/21] fbcon: untangle fbcon_exit

2022-01-31 Thread Daniel Vetter
There's a bunch of confusions going on here:
- The deferred fbcon setup notifier should only be cleaned up from
  fb_console_exit(), to be symmetric with fb_console_init()
- We also need to make sure we don't race with the work, which means
  temporarily dropping the console lock (or we can deadlock)
- That also means no point in clearing deferred_takeover, we are
  unloading everything anyway.
- Finally rename fbcon_exit to fbcon_release_all and move it, since
  that's what's it doing when being called from consw->con_deinit
  through fbcon_deinit.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Greg Kroah-Hartman 
Cc: Claudio Suarez 
Cc: Du Cheng 
Cc: Tetsuo Handa 
---
 drivers/video/fbdev/core/fbcon.c | 63 
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 5c14e24d14a1..22581952b4fd 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -185,7 +185,6 @@ static void fbcon_set_disp(struct fb_info *info, struct 
fb_var_screeninfo *var,
   int unit);
 static void fbcon_modechanged(struct fb_info *info);
 static void fbcon_set_all_vcs(struct fb_info *info);
-static void fbcon_exit(void);
 
 static struct device *fbcon_device;
 
@@ -1149,6 +1148,27 @@ static void fbcon_free_font(struct fbcon_display *p, 
bool freefont)
 
 static void set_vc_hi_font(struct vc_data *vc, bool set);
 
+static void fbcon_release_all(void)
+{
+   struct fb_info *info;
+   int i, j, mapped;
+
+   for_each_registered_fb(i) {
+   mapped = 0;
+   info = registered_fb[i];
+
+   for (j = first_fb_vc; j <= last_fb_vc; j++) {
+   if (con2fb_map[j] == i) {
+   mapped = 1;
+   con2fb_map[j] = -1;
+   }
+   }
+
+   if (mapped)
+   fbcon_release(info);
+   }
+}
+
 static void fbcon_deinit(struct vc_data *vc)
 {
struct fbcon_display *p = &fb_display[vc->vc_num];
@@ -1188,7 +1208,7 @@ static void fbcon_deinit(struct vc_data *vc)
set_vc_hi_font(vc, false);
 
if (!con_is_bound(&fb_con))
-   fbcon_exit();
+   fbcon_release_all();
 
if (vc->vc_num == logo_shown)
logo_shown = FBCON_LOGO_CANSHOW;
@@ -3316,34 +3336,6 @@ static void fbcon_start(void)
 #endif
 }
 
-static void fbcon_exit(void)
-{
-   struct fb_info *info;
-   int i, j, mapped;
-
-#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
-   if (deferred_takeover) {
-   dummycon_unregister_output_notifier(&fbcon_output_nb);
-   deferred_takeover = false;
-   }
-#endif
-
-   for_each_registered_fb(i) {
-   mapped = 0;
-   info = registered_fb[i];
-
-   for (j = first_fb_vc; j <= last_fb_vc; j++) {
-   if (con2fb_map[j] == i) {
-   mapped = 1;
-   con2fb_map[j] = -1;
-   }
-   }
-
-   if (mapped)
-   fbcon_release(info);
-   }
-}
-
 void __init fb_console_init(void)
 {
int i;
@@ -3383,10 +3375,19 @@ static void __exit fbcon_deinit_device(void)
 
 void __exit fb_console_exit(void)
 {
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
+   console_lock();
+   if (deferred_takeover)
+   dummycon_unregister_output_notifier(&fbcon_output_nb);
+   console_unlock();
+
+   cancel_work_sync(&fbcon_deferred_takeover_work);
+#endif
+
console_lock();
fbcon_deinit_device();
device_destroy(fb_class, MKDEV(0, 0));
-   fbcon_exit();
+
do_unregister_con_driver(&fb_con);
console_unlock();
 }  
-- 
2.33.0



[Intel-gfx] [PATCH 19/21] fbcon: Maintain a private array of fb_info

2022-01-31 Thread Daniel Vetter
Accessing the one in fbmem.c without taking the right locks is a bad
idea. Instead maintain our own private copy, which is fully protected
by console_lock() (like everything else in fbcon.c). That copy is
serialized through fbcon_fb_registered/unregistered() calls.

Also this means we do not need to hold a full fb_info reference, which
is nice because doing so would mean a refcount loop between the
console and the fb_info. But it's also not nice since it means
console_lock() must be held absolutely everywhere. Well strictly
speaking we could still try to do some refcounting games again by
calling get_fb_info before we drop the console_lock. But things will
get tricky.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Tetsuo Handa 
Cc: Claudio Suarez 
Cc: Du Cheng 
Cc: Greg Kroah-Hartman 
---
 drivers/video/fbdev/core/fbcon.c | 82 +---
 1 file changed, 43 insertions(+), 39 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 22581952b4fd..a0ca34b29615 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -86,10 +86,6 @@
  * - fbcon state itself is protected by the console_lock, and the code does a
  *   pretty good job at making sure that lock is held everywhere it's needed.
  *
- * - access to the registered_fb array is entirely unprotected. This should use
- *   proper object lifetime handling, i.e. get/put_fb_info. This also means
- *   switching from indices to proper pointers for fb_info everywhere.
- *
  * - fbcon doesn't bother with fb_lock/unlock at all. This is buggy, since it
  *   means concurrent access to the same fbdev from both fbcon and userspace
  *   will blow up. To fix this all fbcon calls from fbmem.c need to be moved 
out
@@ -107,6 +103,13 @@ enum {
 
 static struct fbcon_display fb_display[MAX_NR_CONSOLES];
 
+struct fb_info *fbcon_registered_fb[FB_MAX];
+int fbcon_num_registered_fb;
+
+#define fbcon_for_each_registered_fb(i)\
+   for (i = 0; WARN_CONSOLE_UNLOCKED(), i < FB_MAX; i++)   \
+   if (!fbcon_registered_fb[i]) {} else
+
 static signed char con2fb_map[MAX_NR_CONSOLES];
 static signed char con2fb_map_boot[MAX_NR_CONSOLES];
 
@@ -114,12 +117,7 @@ static struct fb_info *fbcon_info_from_console(int console)
 {
WARN_CONSOLE_UNLOCKED();
 
-   /*
-* Note that only con2fb_map is protected by the console lock,
-* registered_fb is protected by a separate mutex. This lookup can
-* therefore race.
-*/
-   return registered_fb[con2fb_map[console]];
+   return fbcon_registered_fb[con2fb_map[console]];
 }
 
 static int logo_lines;
@@ -516,7 +514,7 @@ static int do_fbcon_takeover(int show_logo)
 {
int err, i;
 
-   if (!num_registered_fb)
+   if (!fbcon_num_registered_fb)
return -ENODEV;
 
if (!show_logo)
@@ -822,7 +820,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
 {
struct vc_data *vc = vc_cons[unit].d;
int oldidx = con2fb_map[unit];
-   struct fb_info *info = registered_fb[newidx];
+   struct fb_info *info = fbcon_registered_fb[newidx];
struct fb_info *oldinfo = NULL;
int found, err = 0, show_logo;
 
@@ -840,7 +838,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
}
 
if (oldidx != -1)
-   oldinfo = registered_fb[oldidx];
+   oldinfo = fbcon_registered_fb[oldidx];
 
found = search_fb_in_map(newidx);
 
@@ -932,13 +930,13 @@ static const char *fbcon_startup(void)
 *  If num_registered_fb is zero, this is a call for the dummy part.
 *  The frame buffer devices weren't initialized yet.
 */
-   if (!num_registered_fb || info_idx == -1)
+   if (!fbcon_num_registered_fb || info_idx == -1)
return display_desc;
/*
 * Instead of blindly using registered_fb[0], we use info_idx, set by
 * fbcon_fb_registered();
 */
-   info = registered_fb[info_idx];
+   info = fbcon_registered_fb[info_idx];
if (!info)
return NULL;

@@ -1153,9 +1151,9 @@ static void fbcon_release_all(void)
struct fb_info *info;
int i, j, mapped;
 
-   for_each_registered_fb(i) {
+   fbcon_for_each_registered_fb(i) {
mapped = 0;
-   info = registered_fb[i];
+   info = fbcon_registered_fb[i];
 
for (j = first_fb_vc; j <= last_fb_vc; j++) {
if (con2fb_map[j] == i) {
@@ -1182,7 +1180,7 @@ static void fbcon_deinit(struct vc_data *vc)
if (idx == -1)
goto finished;
 
-   info = registered_fb[idx];
+   info = fbcon_registered_fb[idx];
 
if (!info)
goto finished;
@@ -2118,9 +2116,9 @@ static int fbcon_switch(struct vc_data *vc)
 *
 * info->currcon = vc->vc_num;
 */
-   for_each_regis

[Intel-gfx] [PATCH 16/21] fbcon: Move console_lock for register/unlink/unregister

2022-01-31 Thread Daniel Vetter
Ideally console_lock becomes an implementation detail of fbcon.c and
doesn't show up anywhere in fbmem.c. We're still pretty far from that,
but at least the register/unregister code is there now.

With this the do_fb_ioctl() handler is the only code in fbmem.c still
calling console_lock().

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Thomas Zimmermann 
Cc: Du Cheng 
Cc: Claudio Suarez 
Cc: Greg Kroah-Hartman 
Cc: Tetsuo Handa 
Cc: Matthew Wilcox 
Cc: Sam Ravnborg 
Cc: Zheyu Ma 
Cc: Guenter Roeck 
Cc: Alex Deucher 
Cc: Zhen Lei 
Cc: Xiyu Yang 
---
 drivers/video/fbdev/core/fbcon.c | 33 ++--
 drivers/video/fbdev/core/fbmem.c | 23 ++
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 11b9f962af6f..e5e8aaf6f60d 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2776,10 +2776,12 @@ void fbcon_fb_unbind(struct fb_info *info)
int i, new_idx = -1;
int idx = info->node;
 
-   WARN_CONSOLE_UNLOCKED();
+   console_lock();
 
-   if (!fbcon_has_console_bind)
+   if (!fbcon_has_console_bind) {
+   console_unlock();
return;
+   }
 
for (i = first_fb_vc; i <= last_fb_vc; i++) {
if (con2fb_map[i] != idx &&
@@ -2814,6 +2816,8 @@ void fbcon_fb_unbind(struct fb_info *info)
}
fbcon_unbind();
}
+
+   console_unlock();
 }
 
 /* called with console_lock held */
@@ -2821,10 +2825,12 @@ void fbcon_fb_unregistered(struct fb_info *info)
 {
int i, idx;
 
-   WARN_CONSOLE_UNLOCKED();
+   console_lock();
 
-   if (deferred_takeover)
+   if (deferred_takeover) {
+   console_unlock();
return;
+   }
 
idx = info->node;
for (i = first_fb_vc; i <= last_fb_vc; i++) {
@@ -2853,6 +2859,7 @@ void fbcon_fb_unregistered(struct fb_info *info)
 
if (!num_registered_fb)
do_unregister_con_driver(&fb_con);
+   console_unlock();
 }
 
 void fbcon_remap_all(struct fb_info *info)
@@ -2910,19 +2917,27 @@ static inline void fbcon_select_primary(struct fb_info 
*info)
 }
 #endif /* CONFIG_FRAMEBUFFER_DETECT_PRIMARY */
 
+static bool lockless_register_fb;
+module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 
0400);
+MODULE_PARM_DESC(lockless_register_fb,
+   "Lockless framebuffer registration for debugging [default=off]");
+
 /* called with console_lock held */
 int fbcon_fb_registered(struct fb_info *info)
 {
int ret = 0, i, idx;
 
-   WARN_CONSOLE_UNLOCKED();
+   if (!lockless_register_fb)
+   console_lock();
+   else
+   atomic_inc(&ignore_console_lock_warning);
 
idx = info->node;
fbcon_select_primary(info);
 
if (deferred_takeover) {
pr_info("fbcon: Deferring console take-over\n");
-   return 0;
+   goto out;
}
 
if (info_idx == -1) {
@@ -2942,6 +2957,12 @@ int fbcon_fb_registered(struct fb_info *info)
}
}
 
+out:
+   if (!lockless_register_fb)
+   console_unlock();
+   else
+   atomic_dec(&ignore_console_lock_warning);
+
return ret;
 }
 
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index fd51d12f2702..904ef1250677 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1573,14 +1573,9 @@ static void do_remove_conflicting_framebuffers(struct 
apertures_struct *a,
}
 }
 
-static bool lockless_register_fb;
-module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 
0400);
-MODULE_PARM_DESC(lockless_register_fb,
-   "Lockless framebuffer registration for debugging [default=off]");
-
 static int do_register_framebuffer(struct fb_info *fb_info)
 {
-   int i, ret;
+   int i;
struct fb_videomode mode;
 
if (fb_check_foreignness(fb_info))
@@ -1649,17 +1644,7 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
}
 #endif
 
-   if (!lockless_register_fb)
-   console_lock();
-   else
-   atomic_inc(&ignore_console_lock_warning);
-   ret = fbcon_fb_registered(fb_info);
-
-   if (!lockless_register_fb)
-   console_unlock();
-   else
-   atomic_dec(&ignore_console_lock_warning);
-   return ret;
+   return fbcon_fb_registered(fb_info);
 }
 
 static void unbind_console(struct fb_info *fb_info)
@@ -1669,9 +1654,7 @@ static void unbind_console(struct fb_info *fb_info)
if (WARN_ON(i < 0 || i >= FB_MAX || registered_fb[i] != fb_info))
return;
 
-   console_lock();
fbcon_fb_unbind(fb_info);
-   console_unlock();
 }
 
 static void unlink_framebuffer(struct fb_info *fb_info)
@@ -1714,9 +1697,7 @@ static v

[Intel-gfx] [PATCH 13/21] fbcon: move more common code into fb_open()

2022-01-31 Thread Daniel Vetter
No idea why con2fb_acquire_newinfo() initializes much less than
fbcon_startup(), but so be it. From a quick look most of the
un-initialized stuff should be fairly harmless, but who knows.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Greg Kroah-Hartman 
Cc: Tetsuo Handa 
Cc: Thomas Zimmermann 
Cc: Claudio Suarez 
Cc: Du Cheng 
---
 drivers/video/fbdev/core/fbcon.c | 74 +---
 1 file changed, 31 insertions(+), 43 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index b83a5a77d8a8..5a3391ff038d 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -680,8 +680,18 @@ static int fbcon_invalid_charcount(struct fb_info *info, 
unsigned charcount)
 
 #endif /* CONFIG_MISC_TILEBLITTING */
 
+static void fbcon_release(struct fb_info *info)
+{
+   if (info->fbops->fb_release)
+   info->fbops->fb_release(info, 0);
+
+   module_put(info->fbops->owner);
+}
+
 static int fbcon_open(struct fb_info *info)
 {
+   struct fbcon_ops *ops;
+
if (!try_module_get(info->fbops->owner))
return -ENODEV;
 
@@ -691,19 +701,22 @@ static int fbcon_open(struct fb_info *info)
return -ENODEV;
}
 
-   return 0;
-}
+   ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
+   if (!ops) {
+   fbcon_release(info);
+   return -ENOMEM;
+   }
 
-static void fbcon_release(struct fb_info *info)
-{
-   if (info->fbops->fb_release)
-   info->fbops->fb_release(info, 0);
+   INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
+   ops->info = info;
+   info->fbcon_par = ops;
+   ops->cur_blink_jiffies = HZ / 5;
 
-   module_put(info->fbops->owner);
+   return 0;
 }
 
 static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
- int unit, int oldidx)
+ int unit)
 {
struct fbcon_ops *ops = NULL;
int err;
@@ -712,27 +725,10 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, 
struct fb_info *info,
if (err)
return err;
 
-   if (!err) {
-   ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
-   if (!ops)
-   err = -ENOMEM;
-
-   INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
-   }
-
-   if (!err) {
-   ops->cur_blink_jiffies = HZ / 5;
-   ops->info = info;
-   info->fbcon_par = ops;
-
-   if (vc)
-   set_blitting_type(vc, info);
-   }
+   ops = info->fbcon_par;
 
-   if (err) {
-   con2fb_map[unit] = oldidx;
-   fbcon_release(info);
-   }
+   if (vc)
+   set_blitting_type(vc, info);
 
return err;
 }
@@ -840,9 +836,11 @@ static int set_con2fb_map(int unit, int newidx, int user)
 
found = search_fb_in_map(newidx);
 
-   con2fb_map[unit] = newidx;
-   if (!err && !found)
-   err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
+   if (!err && !found) {
+   err = con2fb_acquire_newinfo(vc, info, unit);
+   if (!err)
+   con2fb_map[unit] = newidx;
+   }
 
/*
 * If old fb is not mapped to any of the consoles,
@@ -939,20 +937,10 @@ static const char *fbcon_startup(void)
if (fbcon_open(info))
return NULL;
 
-   ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
-   if (!ops) {
-   fbcon_release(info);
-   return NULL;
-   }
-
-   INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
-
+   ops = info->fbcon_par;
ops->currcon = -1;
ops->graphics = 1;
ops->cur_rotate = -1;
-   ops->cur_blink_jiffies = HZ / 5;
-   ops->info = info;
-   info->fbcon_par = ops;
 
p->con_rotate = initial_rotation;
if (p->con_rotate == -1)
@@ -1022,7 +1010,7 @@ static void fbcon_init(struct vc_data *vc, int init)
return;
 
if (!info->fbcon_par)
-   con2fb_acquire_newinfo(vc, info, vc->vc_num, -1);
+   con2fb_acquire_newinfo(vc, info, vc->vc_num);
 
/* If we are not the first console on this
   fb, copy the font from that console */
-- 
2.33.0



[Intel-gfx] [PATCH 14/21] fbcon: use lock_fb_info in fbcon_open/release

2022-01-31 Thread Daniel Vetter
Now we get to the real motiviation, because fbmem.c insists that
that's the right lock for these.

Ofc fbcon.c has a lot more places where it probably should call
lock_fb_info(). But looking at fbmem.c at least most of these seem to
be protected by console_lock() too, which is probably what papers over
any issues.

Note that this means we're shuffling around a bit the locking sections
for some of the console takeover and unbind paths, but not all:
- console binding/unbinding from the console layer never with
lock_fb_info
- unbind (as opposed to unlink) never bother with lock_fb_info

Also the real serialization against set_par and set_pan are still
doing by wrapping the entire ioctl code in console_lock(). So this
shuffling shouldn't be worse than what we had from a "can you trigger
races?" pov, but it's at least clearer.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Claudio Suarez 
Cc: Tetsuo Handa 
Cc: Thomas Zimmermann 
Cc: Greg Kroah-Hartman 
Cc: Du Cheng 
Cc: Sam Ravnborg 
Cc: Matthew Wilcox 
Cc: William Kucharski 
Cc: Alex Deucher 
Cc: Zheyu Ma 
Cc: Zhen Lei 
Cc: Xiyu Yang 
---
 drivers/video/fbdev/core/fbcon.c | 5 +
 drivers/video/fbdev/core/fbmem.c | 4 
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 5a3391ff038d..496bc5f2133e 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -682,8 +682,10 @@ static int fbcon_invalid_charcount(struct fb_info *info, 
unsigned charcount)
 
 static void fbcon_release(struct fb_info *info)
 {
+   lock_fb_info(info);
if (info->fbops->fb_release)
info->fbops->fb_release(info, 0);
+   unlock_fb_info(info);
 
module_put(info->fbops->owner);
 }
@@ -695,11 +697,14 @@ static int fbcon_open(struct fb_info *info)
if (!try_module_get(info->fbops->owner))
return -ENODEV;
 
+   lock_fb_info(info);
if (info->fbops->fb_open &&
info->fbops->fb_open(info, 0)) {
+   unlock_fb_info(info);
module_put(info->fbops->owner);
return -ENODEV;
}
+   unlock_fb_info(info);
 
ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
if (!ops) {
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0fa7ede94fa6..fd51d12f2702 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1653,9 +1653,7 @@ static int do_register_framebuffer(struct fb_info 
*fb_info)
console_lock();
else
atomic_inc(&ignore_console_lock_warning);
-   lock_fb_info(fb_info);
ret = fbcon_fb_registered(fb_info);
-   unlock_fb_info(fb_info);
 
if (!lockless_register_fb)
console_unlock();
@@ -1672,9 +1670,7 @@ static void unbind_console(struct fb_info *fb_info)
return;
 
console_lock();
-   lock_fb_info(fb_info);
fbcon_fb_unbind(fb_info);
-   unlock_fb_info(fb_info);
console_unlock();
 }
 
-- 
2.33.0



[Intel-gfx] [PATCH 10/21] fb: Delete fb_info->queue

2022-01-31 Thread Daniel Vetter
It was only used by fbcon, and that now switched to its own,
private work.

Signed-off-by: Daniel Vetter 
Cc: Helge Deller 
Cc: linux-fb...@vger.kernel.org
---
 include/linux/fb.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 02f362c661c8..a8a00d2ba1f3 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -449,7 +449,6 @@ struct fb_info {
struct fb_var_screeninfo var;   /* Current var */
struct fb_fix_screeninfo fix;   /* Current fix */
struct fb_monspecs monspecs;/* Current Monitor specs */
-   struct work_struct queue;   /* Framebuffer event queue */
struct fb_pixmap pixmap;/* Image hardware mapper */
struct fb_pixmap sprite;/* Cursor hardware mapper */
struct fb_cmap cmap;/* Current cmap */
-- 
2.33.0



[Intel-gfx] [PATCH 11/21] fbcon: Extract fbcon_open/release helpers

2022-01-31 Thread Daniel Vetter
There's two minor behaviour changes in here:
- in error paths we now consistently call fb_ops->fb_release
- fb_release really can't fail (fbmem.c ignores it too) and there's no
  reasonable cleanup we can do anyway.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Claudio Suarez 
Cc: Greg Kroah-Hartman 
Cc: Tetsuo Handa 
Cc: Du Cheng 
---
 drivers/video/fbdev/core/fbcon.c | 107 +++
 1 file changed, 53 insertions(+), 54 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index fa30e1909164..eea2ee14b64c 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -680,19 +680,37 @@ static int fbcon_invalid_charcount(struct fb_info *info, 
unsigned charcount)
 
 #endif /* CONFIG_MISC_TILEBLITTING */
 
+static int fbcon_open(struct fb_info *info)
+{
+   if (!try_module_get(info->fbops->owner))
+   return -ENODEV;
+
+   if (info->fbops->fb_open &&
+   info->fbops->fb_open(info, 0)) {
+   module_put(info->fbops->owner);
+   return -ENODEV;
+   }
+
+   return 0;
+}
+
+static void fbcon_release(struct fb_info *info)
+{
+   if (info->fbops->fb_release)
+   info->fbops->fb_release(info, 0);
+
+   module_put(info->fbops->owner);
+}
 
 static int con2fb_acquire_newinfo(struct vc_data *vc, struct fb_info *info,
  int unit, int oldidx)
 {
struct fbcon_ops *ops = NULL;
-   int err = 0;
-
-   if (!try_module_get(info->fbops->owner))
-   err = -ENODEV;
+   int err;
 
-   if (!err && info->fbops->fb_open &&
-   info->fbops->fb_open(info, 0))
-   err = -ENODEV;
+   err = fbcon_open(info);
+   if (err)
+   return err;
 
if (!err) {
ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
@@ -713,7 +731,7 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, 
struct fb_info *info,
 
if (err) {
con2fb_map[unit] = oldidx;
-   module_put(info->fbops->owner);
+   fbcon_release(info);
}
 
return err;
@@ -724,45 +742,34 @@ static int con2fb_release_oldinfo(struct vc_data *vc, 
struct fb_info *oldinfo,
  int oldidx, int found)
 {
struct fbcon_ops *ops = oldinfo->fbcon_par;
-   int err = 0, ret;
+   int ret;
 
-   if (oldinfo->fbops->fb_release &&
-   oldinfo->fbops->fb_release(oldinfo, 0)) {
-   con2fb_map[unit] = oldidx;
-   if (!found && newinfo->fbops->fb_release)
-   newinfo->fbops->fb_release(newinfo, 0);
-   if (!found)
-   module_put(newinfo->fbops->owner);
-   err = -ENODEV;
-   }
+   fbcon_release(oldinfo);
 
-   if (!err) {
-   fbcon_del_cursor_work(oldinfo);
-   kfree(ops->cursor_state.mask);
-   kfree(ops->cursor_data);
-   kfree(ops->cursor_src);
-   kfree(ops->fontbuffer);
-   kfree(oldinfo->fbcon_par);
-   oldinfo->fbcon_par = NULL;
-   module_put(oldinfo->fbops->owner);
-   /*
- If oldinfo and newinfo are driving the same hardware,
- the fb_release() method of oldinfo may attempt to
- restore the hardware state.  This will leave the
- newinfo in an undefined state. Thus, a call to
- fb_set_par() may be needed for the newinfo.
-   */
-   if (newinfo && newinfo->fbops->fb_set_par) {
-   ret = newinfo->fbops->fb_set_par(newinfo);
+   fbcon_del_cursor_work(oldinfo);
+   kfree(ops->cursor_state.mask);
+   kfree(ops->cursor_data);
+   kfree(ops->cursor_src);
+   kfree(ops->fontbuffer);
+   kfree(oldinfo->fbcon_par);
+   oldinfo->fbcon_par = NULL;
+   /*
+ If oldinfo and newinfo are driving the same hardware,
+ the fb_release() method of oldinfo may attempt to
+ restore the hardware state.  This will leave the
+ newinfo in an undefined state. Thus, a call to
+ fb_set_par() may be needed for the newinfo.
+   */
+   if (newinfo && newinfo->fbops->fb_set_par) {
+   ret = newinfo->fbops->fb_set_par(newinfo);
 
-   if (ret)
-   printk(KERN_ERR "con2fb_release_oldinfo: "
-   "detected unhandled fb_set_par error, "
-   "error code %d\n", ret);
-   }
+   if (ret)
+   printk(KERN_ERR "con2fb_release_oldinfo: "
+   "detected unhandled fb_set_par error, "
+   "error code %d\n", ret);
}
 
-   return err;
+   return 0;
 }
 
 static void con2fb_init_display(str

[Intel-gfx] [PATCH 08/21] fbcon: Use delayed work for cursor

2022-01-31 Thread Daniel Vetter
Allows us to delete a bunch of hand-rolled stuff. Also to simplify the
code we initialize the cursor_work completely when we allocate the
fbcon_ops structure, instead of trying to cope with console
re-initialization.

The motiviation here is that fbcon code stops using the fb_info.queue,
which helps with locking issues around cleanup and all that in a later
patch.

Also note that this allows us to ditch the hand-rolled work cleanup in
fbcon_exit - we already call fbcon_del_cursor_timer, which takes care
of everything. Plus this was racy anyway.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Claudio Suarez 
Cc: Du Cheng 
Cc: Thomas Zimmermann 
Cc: Greg Kroah-Hartman 
Cc: Tetsuo Handa 
---
 drivers/video/fbdev/core/fbcon.c | 85 +---
 drivers/video/fbdev/core/fbcon.h |  4 +-
 2 files changed, 35 insertions(+), 54 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 814b648e8f09..affb40658fee 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -348,8 +348,8 @@ static int get_color(struct vc_data *vc, struct fb_info 
*info,
 
 static void fb_flashcursor(struct work_struct *work)
 {
-   struct fb_info *info = container_of(work, struct fb_info, queue);
-   struct fbcon_ops *ops = info->fbcon_par;
+   struct fbcon_ops *ops = container_of(work, struct fbcon_ops, 
cursor_work.work);
+   struct fb_info *info;
struct vc_data *vc = NULL;
int c;
int mode;
@@ -362,7 +362,10 @@ static void fb_flashcursor(struct work_struct *work)
if (ret == 0)
return;
 
-   if (ops && ops->currcon != -1)
+   /* protected by console_lock */
+   info = ops->info;
+
+   if (ops->currcon != -1)
vc = vc_cons[ops->currcon].d;
 
if (!vc || !con_is_visible(vc) ||
@@ -378,42 +381,25 @@ static void fb_flashcursor(struct work_struct *work)
ops->cursor(vc, info, mode, get_color(vc, info, c, 1),
get_color(vc, info, c, 0));
console_unlock();
-}
 
-static void cursor_timer_handler(struct timer_list *t)
-{
-   struct fbcon_ops *ops = from_timer(ops, t, cursor_timer);
-   struct fb_info *info = ops->info;
-
-   queue_work(system_power_efficient_wq, &info->queue);
-   mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
+   queue_delayed_work(system_power_efficient_wq, &ops->cursor_work,
+  ops->cur_blink_jiffies);
 }
 
-static void fbcon_add_cursor_timer(struct fb_info *info)
+static void fbcon_add_cursor_work(struct fb_info *info)
 {
struct fbcon_ops *ops = info->fbcon_par;
 
-   if ((!info->queue.func || info->queue.func == fb_flashcursor) &&
-   !(ops->flags & FBCON_FLAGS_CURSOR_TIMER) &&
-   !fbcon_cursor_noblink) {
-   if (!info->queue.func)
-   INIT_WORK(&info->queue, fb_flashcursor);
-
-   timer_setup(&ops->cursor_timer, cursor_timer_handler, 0);
-   mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
-   ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
-   }
+   if (!fbcon_cursor_noblink)
+   queue_delayed_work(system_power_efficient_wq, &ops->cursor_work,
+  ops->cur_blink_jiffies);
 }
 
-static void fbcon_del_cursor_timer(struct fb_info *info)
+static void fbcon_del_cursor_work(struct fb_info *info)
 {
struct fbcon_ops *ops = info->fbcon_par;
 
-   if (info->queue.func == fb_flashcursor &&
-   ops->flags & FBCON_FLAGS_CURSOR_TIMER) {
-   del_timer_sync(&ops->cursor_timer);
-   ops->flags &= ~FBCON_FLAGS_CURSOR_TIMER;
-   }
+   cancel_delayed_work_sync(&ops->cursor_work);
 }
 
 #ifndef MODULE
@@ -712,6 +698,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, 
struct fb_info *info,
ops = kzalloc(sizeof(struct fbcon_ops), GFP_KERNEL);
if (!ops)
err = -ENOMEM;
+
+   INIT_DELAYED_WORK(&ops->cursor_work, fb_flashcursor);
}
 
if (!err) {
@@ -749,7 +737,7 @@ static int con2fb_release_oldinfo(struct vc_data *vc, 
struct fb_info *oldinfo,
}
 
if (!err) {
-   fbcon_del_cursor_timer(oldinfo);
+   fbcon_del_cursor_work(oldinfo);
kfree(ops->cursor_state.mask);
kfree(ops->cursor_data);
kfree(ops->cursor_src);
@@ -865,7 +853,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
 logo_shown != FBCON_LOGO_DONTSHOW);
 
if (!found)
-   fbcon_add_cursor_timer(info);
+   fbcon_add_cursor_work(info);
con2fb_map_boot[unit] = newidx;
con2fb_init_display(vc, info, unit, show_logo);
}
@@ -962,6 +950,8 @@ static const char *fbcon_startup(void)
  

[Intel-gfx] [PATCH 09/21] fbcon: Replace FBCON_FLAGS_INIT with a boolean

2022-01-31 Thread Daniel Vetter
It's only one flag and slightly tidier code.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Tetsuo Handa 
Cc: Greg Kroah-Hartman 
Cc: Du Cheng 
Cc: Thomas Zimmermann 
Cc: Claudio Suarez 
---
 drivers/video/fbdev/core/fbcon.c | 11 +--
 drivers/video/fbdev/core/fbcon.h |  4 +---
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index affb40658fee..fa30e1909164 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -773,7 +773,7 @@ static void con2fb_init_display(struct vc_data *vc, struct 
fb_info *info,
 
ops->currcon = fg_console;
 
-   if (info->fbops->fb_set_par && !(ops->flags & FBCON_FLAGS_INIT)) {
+   if (info->fbops->fb_set_par && !ops->initialized) {
ret = info->fbops->fb_set_par(info);
 
if (ret)
@@ -782,7 +782,7 @@ static void con2fb_init_display(struct vc_data *vc, struct 
fb_info *info,
"error code %d\n", ret);
}
 
-   ops->flags |= FBCON_FLAGS_INIT;
+   ops->initialized = true;
ops->graphics = 0;
fbcon_set_disp(info, &info->var, unit);
 
@@ -1101,8 +1101,7 @@ static void fbcon_init(struct vc_data *vc, int init)
 * We need to do it in fbcon_init() to prevent screen corruption.
 */
if (con_is_visible(vc) && vc->vc_mode == KD_TEXT) {
-   if (info->fbops->fb_set_par &&
-   !(ops->flags & FBCON_FLAGS_INIT)) {
+   if (info->fbops->fb_set_par && !ops->initialized) {
ret = info->fbops->fb_set_par(info);
 
if (ret)
@@ -,7 +1110,7 @@ static void fbcon_init(struct vc_data *vc, int init)
"error code %d\n", ret);
}
 
-   ops->flags |= FBCON_FLAGS_INIT;
+   ops->initialized = true;
}
 
ops->graphics = 0;
@@ -1186,7 +1185,7 @@ static void fbcon_deinit(struct vc_data *vc)
if (con_is_visible(vc))
fbcon_del_cursor_work(info);
 
-   ops->flags &= ~FBCON_FLAGS_INIT;
+   ops->initialized = false;
 finished:
 
fbcon_free_font(p, free_font);
diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index dce5ce41093e..b18d0cbf73b6 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -18,8 +18,6 @@
 
 #include 
 
-#define FBCON_FLAGS_INIT 1
-
/*
 *This is the interface between the low-level console driver and the
 *low-level frame buffer device
@@ -77,7 +75,7 @@ struct fbcon_ops {
intblank_state;
intgraphics;
intsave_graphics; /* for debug enter/leave */
-   intflags;
+   bool   initialized;
introtate;
intcur_rotate;
char  *cursor_data;
-- 
2.33.0



[Intel-gfx] [PATCH 12/21] fbcon: Ditch error handling for con2fb_release_oldinfo

2022-01-31 Thread Daniel Vetter
It doesn't ever fail anymore.

Signed-off-by: Daniel Vetter 
Cc: Daniel Vetter 
Cc: Thomas Zimmermann 
Cc: Greg Kroah-Hartman 
Cc: Claudio Suarez 
Cc: Du Cheng 
Cc: Tetsuo Handa 
---
 drivers/video/fbdev/core/fbcon.c | 37 +++-
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index eea2ee14b64c..b83a5a77d8a8 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -737,9 +737,8 @@ static int con2fb_acquire_newinfo(struct vc_data *vc, 
struct fb_info *info,
return err;
 }
 
-static int con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
- struct fb_info *newinfo, int unit,
- int oldidx, int found)
+static void con2fb_release_oldinfo(struct vc_data *vc, struct fb_info *oldinfo,
+  struct fb_info *newinfo)
 {
struct fbcon_ops *ops = oldinfo->fbcon_par;
int ret;
@@ -768,8 +767,6 @@ static int con2fb_release_oldinfo(struct vc_data *vc, 
struct fb_info *oldinfo,
"detected unhandled fb_set_par error, "
"error code %d\n", ret);
}
-
-   return 0;
 }
 
 static void con2fb_init_display(struct vc_data *vc, struct fb_info *info,
@@ -823,7 +820,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
int oldidx = con2fb_map[unit];
struct fb_info *info = registered_fb[newidx];
struct fb_info *oldinfo = NULL;
-   int found, err = 0;
+   int found, err = 0, show_logo;
 
WARN_CONSOLE_UNLOCKED();
 
@@ -852,18 +849,15 @@ static int set_con2fb_map(int unit, int newidx, int user)
 * fbcon should release it.
 */
if (!err && oldinfo && !search_fb_in_map(oldidx))
-   err = con2fb_release_oldinfo(vc, oldinfo, info, unit, oldidx,
-found);
+   con2fb_release_oldinfo(vc, oldinfo, info);
 
-   if (!err) {
-   int show_logo = (fg_console == 0 && !user &&
-logo_shown != FBCON_LOGO_DONTSHOW);
+   show_logo = (fg_console == 0 && !user &&
+logo_shown != FBCON_LOGO_DONTSHOW);
 
-   if (!found)
-   fbcon_add_cursor_work(info);
-   con2fb_map_boot[unit] = newidx;
-   con2fb_init_display(vc, info, unit, show_logo);
-   }
+   if (!found)
+   fbcon_add_cursor_work(info);
+   con2fb_map_boot[unit] = newidx;
+   con2fb_init_display(vc, info, unit, show_logo);
 
if (!search_fb_in_map(info_idx))
info_idx = newidx;
@@ -2786,7 +2780,7 @@ static inline void fbcon_unbind(void) {}
 /* called with console_lock held */
 void fbcon_fb_unbind(struct fb_info *info)
 {
-   int i, new_idx = -1, ret = 0;
+   int i, new_idx = -1;
int idx = info->node;
 
WARN_CONSOLE_UNLOCKED();
@@ -2820,13 +2814,8 @@ void fbcon_fb_unbind(struct fb_info *info)
if (con2fb_map[i] == idx) {
con2fb_map[i] = -1;
if (!search_fb_in_map(idx)) {
-   ret = 
con2fb_release_oldinfo(vc_cons[i].d,
-info, 
NULL, i,
-idx, 0);
-   if (ret) {
-   con2fb_map[i] = idx;
-   return;
-   }
+   con2fb_release_oldinfo(vc_cons[i].d,
+  info, NULL);
}
}
}
-- 
2.33.0



[Intel-gfx] [PATCH 07/21] fbdev/sysfs: Fix locking

2022-01-31 Thread Daniel Vetter
fb_set_var requires we hold the fb_info lock. Or at least this now
matches what the ioctl does ...

Note that ps3fb and sh_mobile_lcdcfb are busted in different ways here,
but I will not fix them up.

Also in practice this isn't a big deal, because really variable fbdev
state is actually protected by console_lock (because fbcon just
doesn't bother with lock_fb_info() at all), and lock_fb_info
protecting anything is really just a neat lie. But that's a much
bigger fish to fry.

Signed-off-by: Daniel Vetter 
Cc: Helge Deller 
Cc: Daniel Vetter 
Cc: Qing Wang 
Cc: Sam Ravnborg 
---
 drivers/video/fbdev/core/fbsysfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/fbdev/core/fbsysfs.c 
b/drivers/video/fbdev/core/fbsysfs.c
index 26892940c213..8c1ee9ecec3d 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -91,9 +91,11 @@ static int activate(struct fb_info *fb_info, struct 
fb_var_screeninfo *var)
 
var->activate |= FB_ACTIVATE_FORCE;
console_lock();
+   lock_fb_info(fb_info);
err = fb_set_var(fb_info, var);
if (!err)
fbcon_update_vcs(fb_info, var->activate & FB_ACTIVATE_ALL);
+   unlock_fb_info(fb_info);
console_unlock();
if (err)
return err;
-- 
2.33.0



[Intel-gfx] [PATCH 05/21] fbcon: Introduce wrapper for console->fb_info lookup

2022-01-31 Thread Daniel Vetter
Half of it is protected by console_lock, but the other half is a lot
more awkward: Registration/deregistration of fbdev are serialized, but
we don't really clear out anything in con2fb_map and so there's
potential for use-after free mixups.

First step is to encapsulate the lookup.

Signed-off-by: Daniel Vetter 
Cc: Helge Deller 
Cc: Daniel Vetter 
Cc: Greg Kroah-Hartman 
Cc: Tetsuo Handa 
Cc: Du Cheng 
Cc: Claudio Suarez 
Cc: Thomas Zimmermann 
---
 drivers/video/fbdev/core/fbcon.c | 76 ++--
 1 file changed, 44 insertions(+), 32 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 2a575620ef59..8f971de35885 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -110,6 +110,18 @@ static struct fbcon_display fb_display[MAX_NR_CONSOLES];
 static signed char con2fb_map[MAX_NR_CONSOLES];
 static signed char con2fb_map_boot[MAX_NR_CONSOLES];
 
+static struct fb_info *fbcon_info_from_console(int console)
+{
+   WARN_CONSOLE_UNLOCKED();
+
+   /*
+* Note that only con2fb_map is protected by the console lock,
+* registered_fb is protected by a separate mutex. This lookup can
+* therefore race.
+*/
+   return registered_fb[con2fb_map[console]];
+}
+
 static int logo_lines;
 /* logo_shown is an index to vc_cons when >= 0; otherwise follows FBCON_LOGO
enums.  */
@@ -197,7 +209,7 @@ static void fbcon_rotate(struct fb_info *info, u32 rotate)
if (!ops || ops->currcon == -1)
return;
 
-   fb_info = registered_fb[con2fb_map[ops->currcon]];
+   fb_info = fbcon_info_from_console(ops->currcon);
 
if (info == fb_info) {
struct fbcon_display *p = &fb_display[ops->currcon];
@@ -224,7 +236,7 @@ static void fbcon_rotate_all(struct fb_info *info, u32 
rotate)
for (i = first_fb_vc; i <= last_fb_vc; i++) {
vc = vc_cons[i].d;
if (!vc || vc->vc_mode != KD_TEXT ||
-   registered_fb[con2fb_map[i]] != info)
+   fbcon_info_from_console(i) != info)
continue;
 
p = &fb_display[vc->vc_num];
@@ -354,7 +366,7 @@ static void fb_flashcursor(struct work_struct *work)
vc = vc_cons[ops->currcon].d;
 
if (!vc || !con_is_visible(vc) ||
-   registered_fb[con2fb_map[vc->vc_num]] != info ||
+   fbcon_info_from_console(vc->vc_num) != info ||
vc->vc_deccm != 1) {
console_unlock();
return;
@@ -789,7 +801,7 @@ static void con2fb_init_display(struct vc_data *vc, struct 
fb_info *info,
if (show_logo) {
struct vc_data *fg_vc = vc_cons[fg_console].d;
struct fb_info *fg_info =
-   registered_fb[con2fb_map[fg_console]];
+   fbcon_info_from_console(fg_console);
 
fbcon_prepare_logo(fg_vc, fg_info, fg_vc->vc_cols,
   fg_vc->vc_rows, fg_vc->vc_cols,
@@ -1012,7 +1024,7 @@ static void fbcon_init(struct vc_data *vc, int init)
if (con2fb_map[vc->vc_num] == -1)
con2fb_map[vc->vc_num] = info_idx;
 
-   info = registered_fb[con2fb_map[vc->vc_num]];
+   info = fbcon_info_from_console(vc->vc_num);
 
if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
logo_shown = FBCON_LOGO_DONTSHOW;
@@ -1231,7 +1243,7 @@ static void fbcon_deinit(struct vc_data *vc)
 static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height,
int width)
 {
-   struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+   struct fb_info *info = fbcon_info_from_console(vc->vc_num);
struct fbcon_ops *ops = info->fbcon_par;
 
struct fbcon_display *p = &fb_display[vc->vc_num];
@@ -1269,7 +1281,7 @@ static void fbcon_clear(struct vc_data *vc, int sy, int 
sx, int height,
 static void fbcon_putcs(struct vc_data *vc, const unsigned short *s,
int count, int ypos, int xpos)
 {
-   struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+   struct fb_info *info = fbcon_info_from_console(vc->vc_num);
struct fbcon_display *p = &fb_display[vc->vc_num];
struct fbcon_ops *ops = info->fbcon_par;
 
@@ -1289,7 +1301,7 @@ static void fbcon_putc(struct vc_data *vc, int c, int 
ypos, int xpos)
 
 static void fbcon_clear_margins(struct vc_data *vc, int bottom_only)
 {
-   struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+   struct fb_info *info = fbcon_info_from_console(vc->vc_num);
struct fbcon_ops *ops = info->fbcon_par;
 
if (!fbcon_is_inactive(vc, info))
@@ -1298,7 +1310,7 @@ static void fbcon_clear_margins(struct vc_data *vc, int 
bottom_only)
 
 static void fbcon_cursor(struct vc_data *vc, int mode)
 {
-   struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
+  

[Intel-gfx] [PATCH 04/21] fbcon: delete a few unneeded forward decl

2022-01-31 Thread Daniel Vetter
I didn't bother with any code movement to fix the others, these just
got a bit in the way.

Signed-off-by: Daniel Vetter 
Cc: Helge Deller 
Cc: Daniel Vetter 
Cc: Thomas Zimmermann 
Cc: Du Cheng 
Cc: Tetsuo Handa 
Cc: Claudio Suarez 
Cc: Greg Kroah-Hartman 
---
 drivers/video/fbdev/core/fbcon.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 39dc18a5de86..2a575620ef59 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -163,18 +163,7 @@ static int fbcon_cursor_noblink;
  *  Interface used by the world
  */
 
-static const char *fbcon_startup(void);
-static void fbcon_init(struct vc_data *vc, int init);
-static void fbcon_deinit(struct vc_data *vc);
-static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height,
-   int width);
-static void fbcon_putc(struct vc_data *vc, int c, int ypos, int xpos);
-static void fbcon_putcs(struct vc_data *vc, const unsigned short *s,
-   int count, int ypos, int xpos);
 static void fbcon_clear_margins(struct vc_data *vc, int bottom_only);
-static void fbcon_cursor(struct vc_data *vc, int mode);
-static int fbcon_switch(struct vc_data *vc);
-static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch);
 static void fbcon_set_palette(struct vc_data *vc, const unsigned char *table);
 
 /*
@@ -184,8 +173,8 @@ static void fbcon_set_disp(struct fb_info *info, struct 
fb_var_screeninfo *var,
   int unit);
 static void fbcon_modechanged(struct fb_info *info);
 static void fbcon_set_all_vcs(struct fb_info *info);
-static void fbcon_start(void);
 static void fbcon_exit(void);
+
 static struct device *fbcon_device;
 
 #ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
-- 
2.33.0



[Intel-gfx] [PATCH 06/21] fbcon: delete delayed loading code

2022-01-31 Thread Daniel Vetter
Before

commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter 
Date:   Tue Aug 1 17:32:07 2017 +0200

fbcon: Make fbcon a built-time depency for fbdev

it was possible to load fbcon and fbdev drivers in any order, which
means that fbcon init had to handle the case where fbdev drivers where
already registered.

This is no longer possible, hence delete that code.

Note that the exit case is a bit more complex and will be done in a
separate patch.

Signed-off-by: Daniel Vetter 
Cc: Helge Deller 
Cc: Daniel Vetter 
Cc: Claudio Suarez 
Cc: Greg Kroah-Hartman 
Cc: Tetsuo Handa 
Cc: Du Cheng 
---
 drivers/video/fbdev/core/fbcon.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 8f971de35885..814b648e8f09 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -942,7 +942,7 @@ static const char *fbcon_startup(void)
return display_desc;
/*
 * Instead of blindly using registered_fb[0], we use info_idx, set by
-* fb_console_init();
+* fbcon_fb_registered();
 */
info = registered_fb[info_idx];
if (!info)
@@ -3316,17 +3316,6 @@ static void fbcon_start(void)
return;
}
 #endif
-
-   if (num_registered_fb) {
-   int i;
-
-   for_each_registered_fb(i) {
-   info_idx = i;
-   break;
-   }
-
-   do_fbcon_takeover(0);
-   }
 }
 
 static void fbcon_exit(void)
-- 
2.33.0



[Intel-gfx] [PATCH 03/21] fbcon: Restore fbcon scrolling acceleration

2022-01-31 Thread Daniel Vetter
This functionally undoes 39aead8373b3 ("fbcon: Disable accelerated
scrolling"), but behind the FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION
option.

References: 
https://lore.kernel.org/dri-devel/feea8303-2b83-fc36-972c-4fc8ad723...@gmx.de/
Fixes: 39aead8373b3 ("fbcon: Disable accelerated scrolling")
Cc: Helge Deller 
Cc:  # v5.11+
Cc: Claudio Suarez 
Cc: Dave Airlie 
Cc: Jani Nikula 
Cc: Linus Torvalds 
Cc: Linux Fbdev development list 
Cc: Pavel Machek 
Cc: Sam Ravnborg 
Cc: Greg Kroah-Hartman 
Cc: Javier Martinez Canillas 
Cc: DRI Development 
Cc: Linux Kernel Mailing List 
Cc: Claudio Suarez 
Cc: Tomi Valkeinen 
Cc: Geert Uytterhoeven 
Cc: Thomas Zimmermann 
Cc: Daniel Vetter 
Cc: Sven Schnelle 
Cc: Gerd Hoffmann 
Signed-off-by: Daniel Vetter 
---
 drivers/video/fbdev/core/fbcon.c | 48 
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 2ff90061c7f3..39dc18a5de86 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -1125,13 +1125,15 @@ static void fbcon_init(struct vc_data *vc, int init)
 
ops->graphics = 0;
 
-   /*
-* No more hw acceleration for fbcon.
-*
-* FIXME: Garbage collect all the now dead code after sufficient time
-* has passed.
-*/
+#ifdef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+   if ((info->flags & FBINFO_HWACCEL_COPYAREA) &&
+   !(info->flags & FBINFO_HWACCEL_DISABLED))
+   p->scrollmode = SCROLL_MOVE;
+   else /* default to something safe */
+   p->scrollmode = SCROLL_REDRAW;
+#else
p->scrollmode = SCROLL_REDRAW;
+#endif
 
/*
 *  ++guenther: console.c:vc_allocate() relies on initializing
@@ -1971,15 +1973,49 @@ static void updatescrollmode(struct fbcon_display *p,
 {
struct fbcon_ops *ops = info->fbcon_par;
int fh = vc->vc_font.height;
+   int cap = info->flags;
+   u16 t = 0;
+   int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep,
+ info->fix.xpanstep);
+   int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t);
int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual,
   info->var.xres_virtual);
+   int good_pan = (cap & FBINFO_HWACCEL_YPAN) &&
+   divides(ypan, vc->vc_font.height) && vyres > yres;
+   int good_wrap = (cap & FBINFO_HWACCEL_YWRAP) &&
+   divides(ywrap, vc->vc_font.height) &&
+   divides(vc->vc_font.height, vyres) &&
+   divides(vc->vc_font.height, yres);
+   int reading_fast = cap & FBINFO_READS_FAST;
+   int fast_copyarea = (cap & FBINFO_HWACCEL_COPYAREA) &&
+   !(cap & FBINFO_HWACCEL_DISABLED);
+   int fast_imageblit = (cap & FBINFO_HWACCEL_IMAGEBLIT) &&
+   !(cap & FBINFO_HWACCEL_DISABLED);
 
p->vrows = vyres/fh;
if (yres > (fh * (vc->vc_rows + 1)))
p->vrows -= (yres - (fh * vc->vc_rows)) / fh;
if ((yres % fh) && (vyres % fh < yres % fh))
p->vrows--;
+
+   if (good_wrap || good_pan) {
+   if (reading_fast || fast_copyarea)
+   p->scrollmode = good_wrap ?
+   SCROLL_WRAP_MOVE : SCROLL_PAN_MOVE;
+   else
+   p->scrollmode = good_wrap ? SCROLL_REDRAW :
+   SCROLL_PAN_REDRAW;
+   } else {
+   if (reading_fast || (fast_copyarea && !fast_imageblit))
+   p->scrollmode = SCROLL_MOVE;
+   else
+   p->scrollmode = SCROLL_REDRAW;
+   }
+
+#ifndef CONFIG_FRAMEBUFFER_CONSOLE_ROTATION
+   p->scrollmode = SCROLL_REDRAW;
+#endif
 }
 
 #define PITCH(w) (((w) + 7) >> 3)
-- 
2.33.0



[Intel-gfx] [PATCH 01/21] MAINTAINERS: Add entry for fbdev core

2022-01-31 Thread Daniel Vetter
Ever since Tomi extracted the core code in 2014 it's been defacto me
maintaining this, with help from others from dri-devel and sometimes
Linus (but those are mostly merge conflicts):

$ git shortlog -ns  drivers/video/fbdev/core/ | head -n5
35  Daniel Vetter
23  Linus Torvalds
10  Hans de Goede
 9  Dave Airlie
 6  Peter Rosin

I think ideally we'd also record that the various firmware fb drivers
(efifb, vesafb, ...) are also maintained in drm-misc because for the
past few years the patches have either been to fix handover issues
with drm drivers, or caused handover issues with drm drivers. So any
other tree just doesn't make sense. But also, there's plenty of
outdated MAINTAINER entries for these with people and git trees that
haven't been active in years, so maybe let's just leave them alone.
And furthermore distros are now adopting simpledrm as the firmware fb
driver, so hopefully the need to care about the fbdev firmware drivers
will go down going forward.

Note that drm-misc is group maintained, I expect that to continue like
we've done before, so no new expectations that patches all go through
my hands. That would be silly. This also means I'm happy to put any
other volunteer's name in the M: line, but otherwise git log says I'm
the one who's stuck with this.

Cc: Dave Airlie 
Cc: Jani Nikula 
Cc: Linus Torvalds 
Cc: Linux Fbdev development list 
Cc: Pavel Machek 
Cc: Sam Ravnborg 
Cc: Greg Kroah-Hartman 
Cc: Javier Martinez Canillas 
Cc: DRI Development 
Cc: Linux Kernel Mailing List 
Cc: Claudio Suarez 
Cc: Tomi Valkeinen 
Cc: Geert Uytterhoeven 
Cc: Thomas Zimmermann 
Cc: Daniel Vetter 
Cc: Sven Schnelle 
Cc: Gerd Hoffmann 
Signed-off-by: Daniel Vetter 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ea3e6c914384..49809eaa3096 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7573,6 +7573,12 @@ S:   Maintained
 W: http://floatingpoint.sourceforge.net/emulator/index.html
 F: arch/x86/math-emu/
 
+FRAMEBUFFER CORE
+M: Daniel Vetter 
+F: drivers/video/fbdev/core/
+S: Odd Fixes
+T: git git://anongit.freedesktop.org/drm/drm-misc
+
 FRAMEBUFFER LAYER
 M: Helge Deller 
 L: linux-fb...@vger.kernel.org
-- 
2.33.0



[Intel-gfx] [PATCH 02/21] fbcon: Resurrect fbcon accelerated scrolling code

2022-01-31 Thread Daniel Vetter
This reverts commit b3ec8cdf457e ("fbdev: Garbage collect fbdev
scrolling acceleration, part 1 (from TODO list)"), but with a twist:

Because distros like fedora&suse (probably more) really want to move
away from fbcon as much as possible, and don't have a need for fancy
accelerated fbcon even less, hide the code behind an option.

There's also been noises about resurrecting the scrollback code and
other pieces, so there's plenty of need for such an option it seems.

Compared to direct revert I did move functions around a bit so they're
all in one block, to minimize the number of #ifdef.

I also tried to stuff this all into a separate file, but that didn't
really look much better. I also considered duplicating fbcon_scroll()
since that's a bit meh now, but again didn't seem worth the trouble.
Maybe when we resurect more code.

And finally to shut up unused parameter warnings warnings the macros
became static inline.

References: 
https://lore.kernel.org/dri-devel/feea8303-2b83-fc36-972c-4fc8ad723...@gmx.de/
Fixes: b3ec8cdf457e ("fbdev: Garbage collect fbdev scrolling acceleration, part 
1 (from TODO list)")
Cc: Claudio Suarez 
Cc:  # v5.16+
Cc: Dave Airlie 
Cc: Jani Nikula 
Cc: Linus Torvalds 
Cc: Linux Fbdev development list 
Cc: Pavel Machek 
Cc: Sam Ravnborg 
Cc: Greg Kroah-Hartman 
Cc: Javier Martinez Canillas 
Cc: DRI Development 
Cc: Linux Kernel Mailing List 
Cc: Claudio Suarez 
Cc: Tomi Valkeinen 
Cc: Geert Uytterhoeven 
Cc: Thomas Zimmermann 
Cc: Daniel Vetter 
Cc: Sven Schnelle 
Cc: Gerd Hoffmann 
Signed-off-by: Daniel Vetter 
---
 Documentation/gpu/todo.rst  |  13 +-
 drivers/video/console/Kconfig   |  11 +
 drivers/video/fbdev/core/bitblit.c  |  16 +
 drivers/video/fbdev/core/fbcon.c| 491 +++-
 drivers/video/fbdev/core/fbcon.h|  59 +++
 drivers/video/fbdev/core/fbcon_ccw.c|  28 +-
 drivers/video/fbdev/core/fbcon_cw.c |  28 +-
 drivers/video/fbdev/core/fbcon_rotate.h |  21 +
 drivers/video/fbdev/core/fbcon_ud.c |  37 +-
 drivers/video/fbdev/core/tileblit.c |  16 +
 drivers/video/fbdev/skeletonfb.c|  12 +-
 include/linux/fb.h  |   2 +-
 12 files changed, 701 insertions(+), 33 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index da138dd39883..29506815d24a 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -303,19 +303,16 @@ Level: Advanced
 Garbage collect fbdev scrolling acceleration
 
 
-Scroll acceleration has been disabled in fbcon. Now it works as the old
-SCROLL_REDRAW mode. A ton of code was removed in fbcon.c and the hook bmove was
-removed from fbcon_ops.
-Remaining tasks:
+Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode =
+SCROLL_REDRAW. There's a ton of code this will allow us to remove:
 
-- a bunch of the hooks in fbcon_ops could be removed or simplified by calling
+- lots of code in fbcon.c
+
+- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called
   directly instead of the function table (with a switch on p->rotate)
 
 - fb_copyarea is unused after this, and can be deleted from all drivers
 
-- after that, fb_copyarea can be deleted from fb_ops in include/linux/fb.h as
-  well as cfb_copyarea
-
 Note that not all acceleration code can be deleted, since clearing and cursor
 support is still accelerated, which might be good candidates for further
 deletion projects.
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 840d9813b0bc..f83f5c755084 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -78,6 +78,17 @@ config FRAMEBUFFER_CONSOLE
help
  Low-level framebuffer-based console driver.
 
+config FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION
+   bool "Enable legacy fbcon code for hw acceleration"
+   depends on FRAMEBUFFER_CONSOLE
+   default n
+   help
+Only enable this options if you are in full control of machine since
+the fbcon acceleration code is essentially unmaintained and known
+problematic.
+
+If unsure, select n.
+
 config FRAMEBUFFER_CONSOLE_DETECT_PRIMARY
bool "Map the console to the primary display device"
depends on FRAMEBUFFER_CONSOLE
diff --git a/drivers/video/fbdev/core/bitblit.c 
b/drivers/video/fbdev/core/bitblit.c
index 01fae2c96965..f98e8f298bc1 100644
--- a/drivers/video/fbdev/core/bitblit.c
+++ b/drivers/video/fbdev/core/bitblit.c
@@ -43,6 +43,21 @@ static void update_attr(u8 *dst, u8 *src, int attribute,
}
 }
 
+static void bit_bmove(struct vc_data *vc, struct fb_info *info, int sy,
+ int sx, int dy, int dx, int height, int width)
+{
+   struct fb_copyarea area;
+
+   area.sx = sx * vc->vc_font.width;
+   area.sy = sy * vc->vc_font.height;
+   area.dx = dx * vc->vc_font.width;
+   area.dy = dy * vc->vc_font.height;
+   area.hei

[Intel-gfx] [PATCH 00/21] some fbcon patches, mostly locking

2022-01-31 Thread Daniel Vetter
Hi all,

This took way longer than I hoped, but well I got lost in head-scratching
locking problems. Anyway ended up typing a pile of fbcon patches. Rough
overview:

- MAINTAINER entry for fbdev core in drm-misc, with the usual group
  maintainering

- The reverts, but with a compile time option. I looked into combining
  this with the RFC from Helge, but it looked like worse than either
  approach, so I've kept mine. I think merging either is fine, no personal
  stake here on the bikeshed color.

- The real distraction, aka a bunch of cleanups and mostly locking work
  for fbcon. I managed to ditch at least one locking FIXME from fbcon.c.

The bad news that I spent a bunch more time pondering about the bigger
locking issues in fbcon.c, and I think as-is they're unfixable. We need
the threaded printk support to land first, so that we're not adding
lock_fb_info() to all printk() contexts. Because that just cannot work.

That also means we'll likely have another fbcon regression to face, in the
form of worse oops printing ability. And that one I don't think we can fix
with clever application of #ifdef, because changing locking rules at
compile time in fundamental ways is just not a very good idea.

Anyway, testing, review, feedback and all that very much welcome, as
usual.

Cheers, Daniel

Daniel Vetter (21):
  MAINTAINERS: Add entry for fbdev core
  fbcon: Resurrect fbcon accelerated scrolling code
  fbcon: Restore fbcon scrolling acceleration
  fbcon: delete a few unneeded forward decl
  fbcon: Introduce wrapper for console->fb_info lookup
  fbcon: delete delayed loading code
  fbdev/sysfs: Fix locking
  fbcon: Use delayed work for cursor
  fbcon: Replace FBCON_FLAGS_INIT with a boolean
  fb: Delete fb_info->queue
  fbcon: Extract fbcon_open/release helpers
  fbcon: Ditch error handling for con2fb_release_oldinfo
  fbcon: move more common code into fb_open()
  fbcon: use lock_fb_info in fbcon_open/release
  fbcon: Consistently protect deferred_takeover with console_lock()
  fbcon: Move console_lock for register/unlink/unregister
  fbcon: Move more code into fbcon_release
  fbcon: untangle fbcon_exit
  fbcon: Maintain a private array of fb_info
  Revert "fbdev: Prevent probing generic drivers if a FB is already
registered"
  fbdev: Make registered_fb[] private to fbmem.c

 Documentation/gpu/todo.rst  |   13 +-
 MAINTAINERS |6 +
 drivers/video/console/Kconfig   |   11 +
 drivers/video/fbdev/core/bitblit.c  |   16 +
 drivers/video/fbdev/core/fbcon.c| 1072 +--
 drivers/video/fbdev/core/fbcon.h|   67 +-
 drivers/video/fbdev/core/fbcon_ccw.c|   28 +-
 drivers/video/fbdev/core/fbcon_cw.c |   28 +-
 drivers/video/fbdev/core/fbcon_rotate.h |   21 +
 drivers/video/fbdev/core/fbcon_ud.c |   37 +-
 drivers/video/fbdev/core/fbmem.c|   35 +-
 drivers/video/fbdev/core/fbsysfs.c  |2 +
 drivers/video/fbdev/core/tileblit.c |   16 +
 drivers/video/fbdev/efifb.c |   11 -
 drivers/video/fbdev/simplefb.c  |   11 -
 drivers/video/fbdev/skeletonfb.c|   12 +-
 include/linux/fb.h  |   10 +-
 17 files changed, 1017 insertions(+), 379 deletions(-)

-- 
2.33.0



[Intel-gfx] ✗ Fi.CI.SPARSE: warning for discrete card 64K page support (rev4)

2022-01-31 Thread Patchwork
== Series Details ==

Series: discrete card 64K page support (rev4)
URL   : https://patchwork.freedesktop.org/series/99119/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.




[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for discrete card 64K page support (rev4)

2022-01-31 Thread Patchwork
== Series Details ==

Series: discrete card 64K page support (rev4)
URL   : https://patchwork.freedesktop.org/series/99119/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5e8b377b5d22 drm/i915: add needs_compact_pt flag
cd0c6d7c7b7b drm/i915: enforce min GTT alignment for discrete cards
-:298: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#298: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:457:
+   if (offset < hole_start + 
aligned_size)

-:310: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#310: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:481:
+   if (offset + aligned_size > 
hole_end)

-:328: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#328: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:497:
+   if (offset < hole_start + 
aligned_size)

-:340: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#340: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:520:
+   if (offset + aligned_size > 
hole_end)

-:358: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#358: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:536:
+   if (offset < hole_start + 
aligned_size)

-:370: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#370: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:560:
+   if (offset + aligned_size > 
hole_end)

-:388: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#388: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:576:
+   if (offset < hole_start + 
aligned_size)

-:400: WARNING:DEEP_INDENTATION: Too many leading tabs - consider code 
refactoring
#400: FILE: drivers/gpu/drm/i915/selftests/i915_gem_gtt.c:599:
+   if (offset + aligned_size > 
hole_end)

total: 0 errors, 8 warnings, 0 checks, 434 lines checked
4605b885e079 drm/i915: support 64K GTT pages for discrete cards
66143948b6a9 drm/i915: add gtt misalignment test
8fa9d39a7902 drm/i915/uapi: document behaviour for DG2 64K support




[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Fix header test and log spam on !x86 (rev2)

2022-01-31 Thread Patchwork
== Series Details ==

Series: drm/i915: Fix header test and log spam on !x86 (rev2)
URL   : https://patchwork.freedesktop.org/series/99344/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11164_full -> Patchwork_22142_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_22142_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22142_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (13 -> 10)
--

  Missing(3): shard-rkl shard-dg1 shard-tglu 

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_22142_full:

### IGT changes ###

 Possible regressions 

  * igt@gem_ctx_persistence@smoketest:
- shard-tglb: [PASS][1] -> [FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-tglb8/igt@gem_ctx_persiste...@smoketest.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-tglb5/igt@gem_ctx_persiste...@smoketest.html

  
Known issues


  Here are the changes found in Patchwork_22142_full that come from known 
issues:

### IGT changes ###

 Issues hit 

  * igt@gem_eio@kms:
- shard-tglb: [PASS][3] -> [FAIL][4] ([i915#232])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-tglb7/igt@gem_...@kms.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-tglb1/igt@gem_...@kms.html

  * igt@gem_exec_capture@pi@rcs0:
- shard-skl:  [PASS][5] -> [INCOMPLETE][6] ([i915#4547])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-skl4/igt@gem_exec_capture@p...@rcs0.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-skl8/igt@gem_exec_capture@p...@rcs0.html

  * igt@gem_exec_fair@basic-flow@rcs0:
- shard-tglb: [PASS][7] -> [FAIL][8] ([i915#2842]) +1 similar issue
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-tglb6/igt@gem_exec_fair@basic-f...@rcs0.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-tglb2/igt@gem_exec_fair@basic-f...@rcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
- shard-iclb: [PASS][9] -> [FAIL][10] ([i915#2849])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-iclb1/igt@gem_exec_fair@basic-throt...@rcs0.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-iclb1/igt@gem_exec_fair@basic-throt...@rcs0.html

  * igt@gem_exec_whisper@basic-forked:
- shard-glk:  [PASS][11] -> [DMESG-WARN][12] ([i915#118]) +2 
similar issues
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-glk8/igt@gem_exec_whis...@basic-forked.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-glk3/igt@gem_exec_whis...@basic-forked.html

  * igt@gem_lmem_swapping@heavy-verify-random:
- shard-skl:  NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4613]) +1 
similar issue
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-skl4/igt@gem_lmem_swapp...@heavy-verify-random.html

  * igt@gem_userptr_blits@input-checking:
- shard-kbl:  NOTRUN -> [DMESG-WARN][14] ([i915#4990])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-kbl3/igt@gem_userptr_bl...@input-checking.html

  * igt@gem_userptr_blits@vma-merge:
- shard-kbl:  NOTRUN -> [FAIL][15] ([i915#3318])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-kbl3/igt@gem_userptr_bl...@vma-merge.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
- shard-apl:  [PASS][16] -> [DMESG-WARN][17] ([i915#180]) +1 
similar issue
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-apl1/igt@i915_susp...@fence-restore-tiled2untiled.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-apl2/igt@i915_susp...@fence-restore-tiled2untiled.html

  * igt@kms_async_flips@alternate-sync-async-flip:
- shard-skl:  [PASS][18] -> [FAIL][19] ([i915#2521])
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/shard-skl4/igt@kms_async_fl...@alternate-sync-async-flip.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-skl8/igt@kms_async_fl...@alternate-sync-async-flip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-hflip:
- shard-skl:  NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#3777])
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/shard-skl4/igt@kms_big...@y-tiled-max-hw-stride-64bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-64bpp-rotate-270:
- shard-iclb: NOTRUN -> [SKIP][21] ([fdo#110723])
   [21]: 
https://intel-gfx

[Intel-gfx] [PATCH v6 5/5] drm/i915/uapi: document behaviour for DG2 64K support

2022-01-31 Thread Robert Beckett
From: Matthew Auld 

On discrete platforms like DG2, we need to support a minimum page size
of 64K when dealing with device local-memory. This is quite tricky for
various reasons, so try to document the new implicit uapi for this.

v3: fix typos and less emphasis
v2: Fixed suggestions on formatting [Daniel]

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
Acked-by: Jordan Justen 
Reviewed-by: Ramalingam C 
Reviewed-by: Thomas Hellström 
cc: Simon Ser 
cc: Pekka Paalanen 
Cc: Jordan Justen 
Cc: Kenneth Graunke 
Cc: mesa-...@lists.freedesktop.org
Cc: Tony Ye 
Cc: Slawomir Milczarek 
---
 include/uapi/drm/i915_drm.h | 44 -
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5e678917da70..77e5e74c32c1 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1118,10 +1118,16 @@ struct drm_i915_gem_exec_object2 {
/**
 * When the EXEC_OBJECT_PINNED flag is specified this is populated by
 * the user with the GTT offset at which this object will be pinned.
+*
 * When the I915_EXEC_NO_RELOC flag is specified this must contain the
 * presumed_offset of the object.
+*
 * During execbuffer2 the kernel populates it with the value of the
 * current GTT offset of the object, for future presumed_offset writes.
+*
+* See struct drm_i915_gem_create_ext for the rules when dealing with
+* alignment restrictions with I915_MEMORY_CLASS_DEVICE, on devices with
+* minimum page sizes, like DG2.
 */
__u64 offset;
 
@@ -3145,11 +3151,39 @@ struct drm_i915_gem_create_ext {
 *
 * The (page-aligned) allocated size for the object will be returned.
 *
-* Note that for some devices we have might have further minimum
-* page-size restrictions(larger than 4K), like for device local-memory.
-* However in general the final size here should always reflect any
-* rounding up, if for example using the 
I915_GEM_CREATE_EXT_MEMORY_REGIONS
-* extension to place the object in device local-memory.
+*
+* DG2 64K min page size implications:
+*
+* On discrete platforms, starting from DG2, we have to contend with GTT
+* page size restrictions when dealing with I915_MEMORY_CLASS_DEVICE
+* objects.  Specifically the hardware only supports 64K or larger GTT
+* page sizes for such memory. The kernel will already ensure that all
+* I915_MEMORY_CLASS_DEVICE memory is allocated using 64K or larger page
+* sizes underneath.
+*
+* Note that the returned size here will always reflect any required
+* rounding up done by the kernel, i.e 4K will now become 64K on devices
+* such as DG2.
+*
+* Special DG2 GTT address alignment requirement:
+*
+* The GTT alignment will also need to be at least 2M for such objects.
+*
+* Note that due to how the hardware implements 64K GTT page support, we
+* have some further complications:
+*
+*   1) The entire PDE (which covers a 2MB virtual address range), must
+*   contain only 64K PTEs, i.e mixing 4K and 64K PTEs in the same
+*   PDE is forbidden by the hardware.
+*
+*   2) We still need to support 4K PTEs for I915_MEMORY_CLASS_SYSTEM
+*   objects.
+*
+* To keep things simple for userland, we mandate that any GTT mappings
+* must be aligned to and rounded up to 2MB. As this only wastes virtual
+* address space and avoids userland having to copy any needlessly
+* complicated PDE sharing scheme (coloring) and only affects DG2, this
+* is deemed to be a good compromise.
 */
__u64 size;
/**
-- 
2.25.1



[Intel-gfx] [PATCH v6 3/5] drm/i915: support 64K GTT pages for discrete cards

2022-01-31 Thread Robert Beckett
From: Matthew Auld 

discrete cards optimise 64K GTT pages for local-memory, since everything
should be allocated at 64K granularity. We say goodbye to sparse
entries, and instead get a compact 256B page-table for 64K pages,
which should be more cache friendly. 4K pages for local-memory
are no longer supported by the HW.

v4: don't return uninitialized err in igt_ppgtt_compact
Reported-by: kernel test robot 

Signed-off-by: Matthew Auld 
Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
Reviewed-by: Thomas Hellström 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
---
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  60 ++
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c  | 108 +-
 drivers/gpu/drm/i915/gt/intel_gtt.h   |   3 +
 drivers/gpu/drm/i915/gt/intel_ppgtt.c |   1 +
 4 files changed, 169 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index f36191ebf964..a7d9bdb85d70 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1478,6 +1478,65 @@ static int igt_ppgtt_sanity_check(void *arg)
return err;
 }
 
+static int igt_ppgtt_compact(void *arg)
+{
+   struct drm_i915_private *i915 = arg;
+   struct drm_i915_gem_object *obj;
+   int err;
+
+   /*
+* Simple test to catch issues with compact 64K pages -- since the pt is
+* compacted to 256B that gives us 32 entries per pt, however since the
+* backing page for the pt is 4K, any extra entries we might incorrectly
+* write out should be ignored by the HW. If ever hit such a case this
+* test should catch it since some of our writes would land in scratch.
+*/
+
+   if (!HAS_64K_PAGES(i915)) {
+   pr_info("device lacks compact 64K page support, skipping\n");
+   return 0;
+   }
+
+   if (!HAS_LMEM(i915)) {
+   pr_info("device lacks LMEM support, skipping\n");
+   return 0;
+   }
+
+   /* We want the range to cover multiple page-table boundaries. */
+   obj = i915_gem_object_create_lmem(i915, SZ_4M, 0);
+   if (IS_ERR(obj))
+   return PTR_ERR(obj);
+
+   err = i915_gem_object_pin_pages_unlocked(obj);
+   if (err)
+   goto out_put;
+
+   if (obj->mm.page_sizes.phys < I915_GTT_PAGE_SIZE_64K) {
+   pr_info("LMEM compact unable to allocate huge-page(s)\n");
+   goto out_unpin;
+   }
+
+   /*
+* Disable 2M GTT pages by forcing the page-size to 64K for the GTT
+* insertion.
+*/
+   obj->mm.page_sizes.sg = I915_GTT_PAGE_SIZE_64K;
+
+   err = igt_write_huge(i915, obj);
+   if (err)
+   pr_err("LMEM compact write-huge failed\n");
+
+out_unpin:
+   i915_gem_object_unpin_pages(obj);
+out_put:
+   i915_gem_object_put(obj);
+
+   if (err == -ENOMEM)
+   err = 0;
+
+   return err;
+}
+
 static int igt_tmpfs_fallback(void *arg)
 {
struct drm_i915_private *i915 = arg;
@@ -1735,6 +1794,7 @@ int i915_gem_huge_page_live_selftests(struct 
drm_i915_private *i915)
SUBTEST(igt_tmpfs_fallback),
SUBTEST(igt_ppgtt_smoke_huge),
SUBTEST(igt_ppgtt_sanity_check),
+   SUBTEST(igt_ppgtt_compact),
};
 
if (!HAS_PPGTT(i915)) {
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index c43e724afa9f..62471730266c 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -233,6 +233,8 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * 
const vm,
   start, end, lvl);
} else {
unsigned int count;
+   unsigned int pte = gen8_pd_index(start, 0);
+   unsigned int num_ptes;
u64 *vaddr;
 
count = gen8_pt_count(start, end);
@@ -242,10 +244,18 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * 
const vm,
atomic_read(&pt->used));
GEM_BUG_ON(!count || count >= atomic_read(&pt->used));
 
+   num_ptes = count;
+   if (pt->is_compact) {
+   GEM_BUG_ON(num_ptes % 16);
+   GEM_BUG_ON(pte % 16);
+   num_ptes /= 16;
+   pte /= 16;
+   }
+
vaddr = px_vaddr(pt);
-   memset64(vaddr + gen8_pd_index(start, 0),
+   memset64(vaddr + pte,
 vm->scratch[0]->encode,
-count);
+num_ptes);
 
  

[Intel-gfx] [PATCH v6 4/5] drm/i915: add gtt misalignment test

2022-01-31 Thread Robert Beckett
add test to check handling of misaligned offsets and sizes

v4:
* remove spurious blank lines
* explicitly cast intel_region_id to intel_memory_type in misaligned_pin
Reported-by: kernel test robot 
v6:
* use NEEDS_COMPACT_PT instead of hard coding for DG2

Signed-off-by: Robert Beckett 
Reviewed-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 128 ++
 1 file changed, 128 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index b80788a2b7f9..c23b1e5cc436 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -22,10 +22,12 @@
  *
  */
 
+#include "gt/intel_gtt.h"
 #include 
 #include 
 
 #include "gem/i915_gem_context.h"
+#include "gem/i915_gem_region.h"
 #include "gem/selftests/mock_context.h"
 #include "gt/intel_context.h"
 #include "gt/intel_gpu_commands.h"
@@ -1067,6 +1069,120 @@ static int shrink_boom(struct i915_address_space *vm,
return err;
 }
 
+static int misaligned_case(struct i915_address_space *vm, struct 
intel_memory_region *mr,
+  u64 addr, u64 size, unsigned long flags)
+{
+   struct drm_i915_gem_object *obj;
+   struct i915_vma *vma;
+   int err = 0;
+   u64 expected_vma_size, expected_node_size;
+
+   obj = i915_gem_object_create_region(mr, size, 0, 0);
+   if (IS_ERR(obj))
+   return PTR_ERR(obj);
+
+   vma = i915_vma_instance(obj, vm, NULL);
+   if (IS_ERR(vma)) {
+   err = PTR_ERR(vma);
+   goto err_put;
+   }
+
+   err = i915_vma_pin(vma, 0, 0, addr | flags);
+   if (err)
+   goto err_put;
+   i915_vma_unpin(vma);
+
+   if (!drm_mm_node_allocated(&vma->node)) {
+   err = -EINVAL;
+   goto err_put;
+   }
+
+   if (i915_vma_misplaced(vma, 0, 0, addr | flags)) {
+   err = -EINVAL;
+   goto err_put;
+   }
+
+   expected_vma_size = round_up(size, 1 << 
(ffs(vma->resource->page_sizes_gtt) - 1));
+   expected_node_size = expected_vma_size;
+
+   if (NEEDS_COMPACT_PT(vm->i915) && i915_gem_object_is_lmem(obj)) {
+   /* compact-pt should expand lmem node to 2MB */
+   expected_vma_size = round_up(size, I915_GTT_PAGE_SIZE_64K);
+   expected_node_size = round_up(size, I915_GTT_PAGE_SIZE_2M);
+   }
+
+   if (vma->size != expected_vma_size || vma->node.size != 
expected_node_size) {
+   err = i915_vma_unbind(vma);
+   err = -EBADSLT;
+   goto err_put;
+   }
+
+   err = i915_vma_unbind(vma);
+   if (err)
+   goto err_put;
+
+   GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
+
+err_put:
+   i915_gem_object_put(obj);
+   cleanup_freed_objects(vm->i915);
+   return err;
+}
+
+static int misaligned_pin(struct i915_address_space *vm,
+ u64 hole_start, u64 hole_end,
+ unsigned long end_time)
+{
+   struct intel_memory_region *mr;
+   enum intel_region_id id;
+   unsigned long flags = PIN_OFFSET_FIXED | PIN_USER;
+   int err = 0;
+   u64 hole_size = hole_end - hole_start;
+
+   if (i915_is_ggtt(vm))
+   flags |= PIN_GLOBAL;
+
+   for_each_memory_region(mr, vm->i915, id) {
+   u64 min_alignment = i915_vm_min_alignment(vm, (enum 
intel_memory_type)id);
+   u64 size = min_alignment;
+   u64 addr = round_up(hole_start + (hole_size / 2), 
min_alignment);
+
+   /* we can't test < 4k alignment due to flags being encoded in 
lower bits */
+   if (min_alignment != I915_GTT_PAGE_SIZE_4K) {
+   err = misaligned_case(vm, mr, addr + (min_alignment / 
2), size, flags);
+   /* misaligned should error with -EINVAL*/
+   if (!err)
+   err = -EBADSLT;
+   if (err != -EINVAL)
+   return err;
+   }
+
+   /* test for vma->size expansion to min page size */
+   err = misaligned_case(vm, mr, addr, PAGE_SIZE, flags);
+   if (min_alignment > hole_size) {
+   if (!err)
+   err = -EBADSLT;
+   else if (err == -ENOSPC)
+   err = 0;
+   }
+   if (err)
+   return err;
+
+   /* test for intermediate size not expanding vma->size for large 
alignments */
+   err = misaligned_case(vm, mr, addr, size / 2, flags);
+   if (min_alignment > hole_size) {
+   if (!err)
+   err = -EBADSLT;
+   else if (err == -ENOSPC)
+   err = 0;
+

[Intel-gfx] [PATCH v6 2/5] drm/i915: enforce min GTT alignment for discrete cards

2022-01-31 Thread Robert Beckett
From: Matthew Auld 

For local-memory objects we need to align the GTT addresses
to 64K, both for the ppgtt and ggtt.

We need to support vm->min_alignment > 4K, depending
on the vm itself and the type of object we are inserting.
With this in mind update the GTT selftests to take this
into account.

For compact-pt we further align and pad lmem object GTT addresses
to 2MB to ensure PDEs contain consistent page sizes as
required by the HW.

v3:
* use needs_compact_pt flag to discriminate between
  64K and 64K with compact-pt
* add i915_vm_obj_min_alignment
* use i915_vm_obj_min_alignment to round up vma reservation
  if compact-pt instead of hard coding
v5:
* fix i915_vm_obj_min_alignment for internal objects which
  have no memory region
v6:
* tiled_blits_create correctly pick largest required alignment

Signed-off-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
Reviewed-by: Thomas Hellström 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
---
 .../i915/gem/selftests/i915_gem_client_blt.c  | 21 ++--
 drivers/gpu/drm/i915/gt/intel_gtt.c   | 12 +++
 drivers/gpu/drm/i915/gt/intel_gtt.h   | 18 
 drivers/gpu/drm/i915/i915_vma.c   |  9 ++
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 96 ---
 5 files changed, 115 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
index c8ff8bf0986d..3675d12a7d9a 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_client_blt.c
@@ -39,6 +39,7 @@ struct tiled_blits {
struct blit_buffer scratch;
struct i915_vma *batch;
u64 hole;
+   u64 align;
u32 width;
u32 height;
 };
@@ -410,14 +411,19 @@ tiled_blits_create(struct intel_engine_cs *engine, struct 
rnd_state *prng)
goto err_free;
}
 
-   hole_size = 2 * PAGE_ALIGN(WIDTH * HEIGHT * 4);
+   t->align = i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_LOCAL);
+   t->align = max(t->align,
+  i915_vm_min_alignment(t->ce->vm, INTEL_MEMORY_SYSTEM));
+
+   hole_size = 2 * round_up(WIDTH * HEIGHT * 4, t->align);
hole_size *= 2; /* room to maneuver */
-   hole_size += 2 * I915_GTT_MIN_ALIGNMENT;
+   hole_size += 2 * t->align; /* padding on either side */
 
mutex_lock(&t->ce->vm->mutex);
memset(&hole, 0, sizeof(hole));
err = drm_mm_insert_node_in_range(&t->ce->vm->mm, &hole,
- hole_size, 0, I915_COLOR_UNEVICTABLE,
+ hole_size, t->align,
+ I915_COLOR_UNEVICTABLE,
  0, U64_MAX,
  DRM_MM_INSERT_BEST);
if (!err)
@@ -428,7 +434,7 @@ tiled_blits_create(struct intel_engine_cs *engine, struct 
rnd_state *prng)
goto err_put;
}
 
-   t->hole = hole.start + I915_GTT_MIN_ALIGNMENT;
+   t->hole = hole.start + t->align;
pr_info("Using hole at %llx\n", t->hole);
 
err = tiled_blits_create_buffers(t, WIDTH, HEIGHT, prng);
@@ -455,7 +461,7 @@ static void tiled_blits_destroy(struct tiled_blits *t)
 static int tiled_blits_prepare(struct tiled_blits *t,
   struct rnd_state *prng)
 {
-   u64 offset = PAGE_ALIGN(t->width * t->height * 4);
+   u64 offset = round_up(t->width * t->height * 4, t->align);
u32 *map;
int err;
int i;
@@ -486,8 +492,7 @@ static int tiled_blits_prepare(struct tiled_blits *t,
 
 static int tiled_blits_bounce(struct tiled_blits *t, struct rnd_state *prng)
 {
-   u64 offset =
-   round_up(t->width * t->height * 4, 2 * I915_GTT_MIN_ALIGNMENT);
+   u64 offset = round_up(t->width * t->height * 4, 2 * t->align);
int err;
 
/* We want to check position invariant tiling across GTT eviction */
@@ -500,7 +505,7 @@ static int tiled_blits_bounce(struct tiled_blits *t, struct 
rnd_state *prng)
 
/* Reposition so that we overlap the old addresses, and slightly off */
err = tiled_blit(t,
-&t->buffers[2], t->hole + I915_GTT_MIN_ALIGNMENT,
+&t->buffers[2], t->hole + t->align,
 &t->buffers[1], t->hole + 3 * offset / 2);
if (err)
return err;
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 46be4197b93f..df23ebdfc994 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -223,6 +223,18 @@ void i915_address_space_init(struct i915_address_space 
*vm, int subclass)
 
GEM_BUG_ON(!vm->total);
drm_mm_init(&vm->mm, 0, vm->total);
+
+   memset64(vm->min_alignment, I91

[Intel-gfx] [PATCH v6 1/5] drm/i915: add needs_compact_pt flag

2022-01-31 Thread Robert Beckett
From: Ramalingam C 

Add a new platform flag, needs_compact_pt, to mark the requirement of
compact pt layout support for the ppGTT when using 64K GTT pages.

With this flag has_64k_pages will only indicate requirement of 64K
GTT page sizes or larger for device local memory access.

v6:
* minor doc formatting

Suggested-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
Reviewed-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/i915_drv.h  | 11 ---
 drivers/gpu/drm/i915/i915_pci.c  |  2 ++
 drivers/gpu/drm/i915/intel_device_info.h |  1 +
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 00e7594b59c9..4afdfa5fd3b3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1512,12 +1512,17 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 /*
  * Set this flag, when platform requires 64K GTT page sizes or larger for
- * device local memory access. Also this flag implies that we require or
- * at least support the compact PT layout for the ppGTT when using the 64K
- * GTT pages.
+ * device local memory access.
  */
 #define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages)
 
+/*
+ * Set this flag when platform doesn't allow both 64k pages and 4k pages in
+ * the same PT. this flag means we need to support compact PT layout for the
+ * ppGTT when using the 64K GTT pages.
+ */
+#define NEEDS_COMPACT_PT(dev_priv) (INTEL_INFO(dev_priv)->needs_compact_pt)
+
 #define HAS_IPC(dev_priv)   (INTEL_INFO(dev_priv)->display.has_ipc)
 
 #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 2df2db0a5d70..ce6ae6a3cbdf 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1028,6 +1028,7 @@ static const struct intel_device_info xehpsdv_info = {
PLATFORM(INTEL_XEHPSDV),
.display = { },
.has_64k_pages = 1,
+   .needs_compact_pt = 1,
.platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |
BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) |
@@ -1046,6 +1047,7 @@ static const struct intel_device_info dg2_info = {
PLATFORM(INTEL_DG2),
.has_guc_deprivilege = 1,
.has_64k_pages = 1,
+   .needs_compact_pt = 1,
.platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |
BIT(VECS0) | BIT(VECS1) |
diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
b/drivers/gpu/drm/i915/intel_device_info.h
index abf1e103c558..d8da40d01dca 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -130,6 +130,7 @@ enum intel_ppgtt_type {
/* Keep has_* in alphabetical order */ \
func(has_64bit_reloc); \
func(has_64k_pages); \
+   func(needs_compact_pt); \
func(gpu_reset_clobbers_display); \
func(has_reset_engine); \
func(has_global_mocs); \
-- 
2.25.1



[Intel-gfx] [PATCH v6 0/5] discrete card 64K page support

2022-01-31 Thread Robert Beckett
This series continues support for 64K pages for discrete cards.
It supersedes the 64K patches from 
https://patchwork.freedesktop.org/series/95686/#rev4
Changes since that series:

- set min alignment for DG2 to 2MB in i915_address_space_init
- replace coloring with simpler 2MB VA alignment for lmem buffers
- enforce alignment to 2MB for lmem objects on DG2 in i915_vma_insert
- expand vma reservation to round up to 2MB on DG2 in i915_vma_insert
- add alignment test

v2: rebase and fix for async vma that landed
v3:
* fix uapi doc typos
* add needs_compact_pt flag patch
* cleanup vma expansion to use vm->min_alignment instead of hard coding
v4:
* fix err return in igt_ppgtt_compact test
* placate ci robot with explicit enum conversion in misaligned_pin
* remove some blank lines
v5:
* fix obj alignment requirements querying for internal buffers that
  have no memory region associated. (fixes v3 bug)
v6:
* use NEEDS_COMPACT_PT inead of hard coding in misalignment test
* tiled_blits_create correctly pick largest required alignment
* minor doc formatting

Reviewed-by: Thomas Hellström 

Matthew Auld (3):
  drm/i915: enforce min GTT alignment for discrete cards
  drm/i915: support 64K GTT pages for discrete cards
  drm/i915/uapi: document behaviour for DG2 64K support

Ramalingam C (1):
  drm/i915: add needs_compact_pt flag

Robert Beckett (1):
  drm/i915: add gtt misalignment test

 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  60 +
 .../i915/gem/selftests/i915_gem_client_blt.c  |  21 +-
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c  | 108 -
 drivers/gpu/drm/i915/gt/intel_gtt.c   |  12 +
 drivers/gpu/drm/i915/gt/intel_gtt.h   |  21 ++
 drivers/gpu/drm/i915/gt/intel_ppgtt.c |   1 +
 drivers/gpu/drm/i915/i915_drv.h   |  11 +-
 drivers/gpu/drm/i915/i915_pci.c   |   2 +
 drivers/gpu/drm/i915/i915_vma.c   |   9 +
 drivers/gpu/drm/i915/intel_device_info.h  |   1 +
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 224 +++---
 include/uapi/drm/i915_drm.h   |  44 +++-
 12 files changed, 462 insertions(+), 52 deletions(-)

-- 
2.25.1



Re: [Intel-gfx] [PATCH v2 06/17] drm/i915: Pass crtc+cpu_transcoder to intel_cpu_transcoder_set_m_n()

2022-01-31 Thread Ville Syrjälä
On Mon, Jan 31, 2022 at 08:29:32PM +0200, Ville Syrjälä wrote:
> On Mon, Jan 31, 2022 at 04:37:00PM +0200, Jani Nikula wrote:
> > > @@ -2508,15 +2509,16 @@ static void i9xx_crtc_enable(struct 
> > > intel_atomic_state *state,
> > >   const struct intel_crtc_state *new_crtc_state =
> > >   intel_atomic_get_new_crtc_state(state, crtc);
> > >   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > + enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
> > >   enum pipe pipe = crtc->pipe;
> > >  
> > >   if (drm_WARN_ON(&dev_priv->drm, crtc->active))
> > >   return;
> > >  
> > >   if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> > > - intel_cpu_transcoder_set_m1_n1(new_crtc_state,
> > > + intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
> > >  &new_crtc_state->dp_m_n);
> > > - intel_cpu_transcoder_set_m2_n2(new_crtc_state,
> > > + intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
> > 
> > m1_n1 copy paste fail?
> 
> Yep. I guess we don't have g4x+DP in CI so this went unnoticed.

And it wouldn't have helped to have one since the introduction of 
i9xx_configure_cpu_transcoder() in pathc 9 already fixed this.
So the only way to hit it would have been to bisect through the
series.

-- 
Ville Syrjälä
Intel


Re: [Intel-gfx] [PATCH v2 12/17] drm/i915: Fix intel_cpu_transcoder_has_m2_n2()

2022-01-31 Thread Ville Syrjälä
On Mon, Jan 31, 2022 at 05:05:53PM +0200, Jani Nikula wrote:
> On Fri, 28 Jan 2022, Ville Syrjala  wrote:
> > From: Ville Syrjälä 
> >
> > M2/N2 values are present for all ilk-ivb,vlv,chv (and hsw edp).
> > Make the code reflect that.
> 
> Nitpick, it's not called intel_cpu_transcoder_has_m2_n2() until in the
> next patch.
> 
> Side note, I've also been looking at this bit in intel_drrs_set_state():
> 
>   if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv))
>   intel_drrs_set_refresh_rate_m_n(crtc_state, refresh_type);
>   else if (DISPLAY_VER(dev_priv) > 6)
>   intel_drrs_set_refresh_rate_pipeconf(crtc_state, refresh_type);
> 
> and wondering if that should be deduplicated with the
> transcoder_has_m2_n2() somehow. This is all a bit confusing with the
> slightly different conditions.

Yeah, I have a patch to use intel_cpu_transcoder_has_m2_n2() for
this already on my drrs branch. It just didn't make the cut for
this series for some arbitrary reason.

The other place we could perhaps use intel_cpu_transcoder_has_m2_n2()
is the PIPE_CONF_CHECK_ALT vs. checking both m_n and m2_n2. But I don't
really want the logic there to depend on the states it's trying to
compare, so I think a naked platform check there is better.

-- 
Ville Syrjälä
Intel


Re: [Intel-gfx] [PATCH v2 06/17] drm/i915: Pass crtc+cpu_transcoder to intel_cpu_transcoder_set_m_n()

2022-01-31 Thread Ville Syrjälä
On Mon, Jan 31, 2022 at 04:37:00PM +0200, Jani Nikula wrote:
> > @@ -2508,15 +2509,16 @@ static void i9xx_crtc_enable(struct 
> > intel_atomic_state *state,
> > const struct intel_crtc_state *new_crtc_state =
> > intel_atomic_get_new_crtc_state(state, crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +   enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
> > enum pipe pipe = crtc->pipe;
> >  
> > if (drm_WARN_ON(&dev_priv->drm, crtc->active))
> > return;
> >  
> > if (intel_crtc_has_dp_encoder(new_crtc_state)) {
> > -   intel_cpu_transcoder_set_m1_n1(new_crtc_state,
> > +   intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
> >&new_crtc_state->dp_m_n);
> > -   intel_cpu_transcoder_set_m2_n2(new_crtc_state,
> > +   intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
> 
> m1_n1 copy paste fail?

Yep. I guess we don't have g4x+DP in CI so this went unnoticed. I'll
give it a quick smoke test on my ctg+DP to make sure nothing else
slipped through.

-- 
Ville Syrjälä
Intel


[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix header test and log spam on !x86 (rev2)

2022-01-31 Thread Patchwork
== Series Details ==

Series: drm/i915: Fix header test and log spam on !x86 (rev2)
URL   : https://patchwork.freedesktop.org/series/99344/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11164 -> Patchwork_22142


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/index.html

Participating hosts (48 -> 43)
--

  Missing(5): shard-tglu fi-tgl-1115g4 fi-bsw-cyan shard-rkl fi-bdw-samus 

Known issues


  Here are the changes found in Patchwork_22142 that come from known issues:

### IGT changes ###

 Issues hit 

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
- fi-snb-2600:NOTRUN -> [SKIP][1] ([fdo#109271]) +17 similar issues
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-snb-2600/igt@amdgpu/amd_cs_...@sync-fork-compute0.html

  * igt@gem_exec_suspend@basic-s3@smem:
- fi-skl-6600u:   NOTRUN -> [INCOMPLETE][2] ([i915#4547])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-skl-6600u/igt@gem_exec_suspend@basic...@smem.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
- fi-hsw-4770:[PASS][3] -> [SKIP][4] ([fdo#109271])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/fi-hsw-4770/igt@i915_pm_...@basic-pci-d3-state.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-hsw-4770/igt@i915_pm_...@basic-pci-d3-state.html

  * igt@i915_selftest@live@hangcheck:
- fi-hsw-4770:[PASS][5] -> [INCOMPLETE][6] ([i915#3303])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-hsw-4770/igt@i915_selftest@l...@hangcheck.html

  * igt@runner@aborted:
- fi-hsw-4770:NOTRUN -> [FAIL][7] ([fdo#109271] / [i915#1436] / 
[i915#4312])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-hsw-4770/igt@run...@aborted.html
- fi-skl-6600u:   NOTRUN -> [FAIL][8] ([i915#4312])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-skl-6600u/igt@run...@aborted.html

  
 Possible fixes 

  * igt@i915_selftest@live@hangcheck:
- bat-dg1-6:  [DMESG-FAIL][9] ([i915#4494] / [i915#4957]) -> 
[PASS][10]
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/bat-dg1-6/igt@i915_selftest@l...@hangcheck.html
- fi-snb-2600:[INCOMPLETE][11] ([i915#3921]) -> [PASS][12]
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-snb-2600/igt@i915_selftest@l...@hangcheck.html

  * igt@kms_frontbuffer_tracking@basic:
- fi-cfl-8109u:   [DMESG-FAIL][13] ([i915#295]) -> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-cfl-8109u/igt@kms_frontbuffer_track...@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-cfl-8109u:   [DMESG-WARN][15] ([i915#295]) -> [PASS][16] +10 
similar issues
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11164/fi-cfl-8109u/igt@kms_pipe_crc_ba...@read-crc-pipe-b.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22142/fi-cfl-8109u/igt@kms_pipe_crc_ba...@read-crc-pipe-b.html

  
  {name}: This element is suppressed. This means it is ignored when computing
  the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957


Build changes
-

  * Linux: CI_DRM_11164 -> Patchwork_22142

  CI-20190529: 20190529
  CI_DRM_11164: b0839553cd93d73bf43e0949203b24044113b5e7 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6337: 7c9c034619ef9dbfbfe041fbf3973a1cf1ac7a22 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22142: 4e6cbaa5e08a9d0f53d55d94ee0a20c9bb7c6f89 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4e6cbaa5e08a drm/i915: Do not spam log with missing arch support
3efe

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/dg2: s/engine->i915/i915/ for engine workarounds

2022-01-31 Thread Matt Roper
On Sat, Jan 29, 2022 at 12:51:42AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/dg2: s/engine->i915/i915/ for engine workarounds
> URL   : https://patchwork.freedesktop.org/series/99484/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_11159_full -> Patchwork_22139_full
> 
> 
> Summary
> ---
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_22139_full absolutely need to 
> be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_22139_full, please notify your bug team to allow 
> them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Participating hosts (10 -> 10)
> --
> 
>   No changes in participating hosts
> 
> Possible new issues
> ---
> 
>   Here are the unknown changes that may have been introduced in 
> Patchwork_22139_full:
> 
> ### IGT changes ###
> 
>  Possible regressions 
> 
>   * igt@drm_mm@all@insert_range:
> - shard-skl:  NOTRUN -> [INCOMPLETE][1]
>[1]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-skl8/igt@drm_mm@all@insert_range.html

Looks like https://gitlab.freedesktop.org/drm/intel/-/issues/2485

Not related to the changes in this series.  Patch applied to
drm-intel-gt-next.  Thanks Tvrtko for the review.


Matt

> 
>   
> Known issues
> 
> 
>   Here are the changes found in Patchwork_22139_full that come from known 
> issues:
> 
> ### IGT changes ###
> 
>  Issues hit 
> 
>   * igt@gem_exec_balancer@parallel-keep-submit-fence:
> - shard-iclb: [PASS][2] -> [SKIP][3] ([i915#4525]) +1 similar 
> issue
>[2]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11159/shard-iclb1/igt@gem_exec_balan...@parallel-keep-submit-fence.html
>[3]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-iclb7/igt@gem_exec_balan...@parallel-keep-submit-fence.html
> 
>   * igt@gem_exec_fair@basic-deadline:
> - shard-skl:  NOTRUN -> [FAIL][4] ([i915#2846])
>[4]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-skl4/igt@gem_exec_f...@basic-deadline.html
> 
>   * igt@gem_exec_fair@basic-none-share@rcs0:
> - shard-iclb: [PASS][5] -> [FAIL][6] ([i915#2842]) +2 similar 
> issues
>[5]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11159/shard-iclb6/igt@gem_exec_fair@basic-none-sh...@rcs0.html
>[6]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-iclb7/igt@gem_exec_fair@basic-none-sh...@rcs0.html
> 
>   * igt@gem_exec_fair@basic-none@rcs0:
> - shard-glk:  [PASS][7] -> [FAIL][8] ([i915#2842])
>[7]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11159/shard-glk5/igt@gem_exec_fair@basic-n...@rcs0.html
>[8]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-glk4/igt@gem_exec_fair@basic-n...@rcs0.html
> 
>   * igt@gem_exec_fair@basic-none@vcs0:
> - shard-kbl:  [PASS][9] -> [FAIL][10] ([i915#2842])
>[9]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11159/shard-kbl4/igt@gem_exec_fair@basic-n...@vcs0.html
>[10]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-kbl6/igt@gem_exec_fair@basic-n...@vcs0.html
> 
>   * igt@gem_exec_fair@basic-none@vcs1:
> - shard-iclb: NOTRUN -> [FAIL][11] ([i915#2842])
>[11]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-iclb2/igt@gem_exec_fair@basic-n...@vcs1.html
> 
>   * igt@gem_exec_schedule@smoketest@vecs0:
> - shard-skl:  [PASS][12] -> [DMESG-WARN][13] ([i915#1982])
>[12]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11159/shard-skl6/igt@gem_exec_schedule@smoket...@vecs0.html
>[13]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-skl7/igt@gem_exec_schedule@smoket...@vecs0.html
> 
>   * igt@gem_exec_schedule@u-semaphore-user:
> - shard-snb:  NOTRUN -> [SKIP][14] ([fdo#109271]) +77 similar 
> issues
>[14]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-snb4/igt@gem_exec_sched...@u-semaphore-user.html
> 
>   * igt@gem_lmem_swapping@verify-random:
> - shard-skl:  NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#4613]) 
> +1 similar issue
>[15]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-skl9/igt@gem_lmem_swapp...@verify-random.html
> 
>   * igt@gem_pxp@regular-baseline-src-copy-readible:
> - shard-kbl:  NOTRUN -> [SKIP][16] ([fdo#109271]) +48 similar 
> issues
>[16]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-kbl1/igt@gem_...@regular-baseline-src-copy-readible.html
> 
>   * igt@gem_userptr_blits@input-checking:
> - shard-kbl:  NOTRUN -> [DMESG-WARN][17] ([i915#4990])
>[17]: 
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22139/shard-kbl1/igt@gem_userptr

Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/: Re-work clflush_write32

2022-01-31 Thread Michael Cheng

Hey Tvrtko,

Are you saying when adding drm_clflush_virt_range(addr, sizeof(addr), 
this function forces an x86 code path only? If that is the case, 
drm_clflush_virt_range(addr, sizeof(addr) currently has ifdefs that 
seperate out x86 and powerpc, so we can add an ifdef for arm in the near 
future when needed.


On 2022-01-31 6:55 a.m., Tvrtko Ursulin wrote:

On 28/01/2022 22:10, Michael Cheng wrote:

Use drm_clflush_virt_range instead of clflushopt and remove the memory
barrier, since drm_clflush_virt_range takes care of that.

Signed-off-by: Michael Cheng 
---
  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

index 498b458fd784..0854276ff7ba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1332,10 +1332,8 @@ static void *reloc_vaddr(struct i915_vma *vma,
  static void clflush_write32(u32 *addr, u32 value, unsigned int 
flushes)

  {
  if (unlikely(flushes & (CLFLUSH_BEFORE | CLFLUSH_AFTER))) {
-    if (flushes & CLFLUSH_BEFORE) {
-    clflushopt(addr);
-    mb();
-    }
+    if (flushes & CLFLUSH_BEFORE)
+    drm_clflush_virt_range(addr, sizeof(addr));
    *addr = value;
  @@ -1347,7 +1345,7 @@ static void clflush_write32(u32 *addr, u32 
value, unsigned int flushes)

   * to ensure ordering of clflush wrt to the system.
   */
  if (flushes & CLFLUSH_AFTER)
-    clflushopt(addr);
+    drm_clflush_virt_range(addr, sizeof(addr));
  } else
  *addr = value;
  }


Slightly annoying thing here (maybe in some other patches from the 
series as well) is that the change adds a function call to x86 only 
code path, because relocations are not supported on discrete as per:


static in
eb_validate_vma(...)
    /* Relocations are disallowed for all platforms after TGL-LP.  
This

 * also covers all platforms with local memory.
 */

    if (entry->relocation_count &&
    GRAPHICS_VER(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915))
    return -EINVAL;

How acceptable would be, for the whole series, to introduce a static 
inline i915 cluflush wrapper and so be able to avoid functions calls 
on x86? Is this something that has been discussed and discounted already?


Regards,

Tvrtko

P.S. Hmm I am now reminded of my really old per platform build 
patches. With them you would be able to compile out large portions of 
the driver when building for ARM. Probably like a 3rd if my memory 
serves me right.


[Intel-gfx] [PATCH v3 1/3] drm: Stop spamming log with drm_cache message

2022-01-31 Thread Lucas De Marchi
Only x86 and in some cases PPC have support added in drm_cache.c for the
clflush class of functions. However warning once is sufficient to taint
the log instead of spamming it with "Architecture has no drm_cache.c
support" every few millisecond. Switch to WARN_ONCE() so we still get
the log message, but only once, together with the warning. E.g:

[ cut here ]
Architecture has no drm_cache.c support
WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 
drm_clflush_sg+0x40/0x50 [drm]
...

v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err()

Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Signed-off-by: Lucas De Marchi 
---

v3: No changes from previous version, just submitting to the right
mailing list

 drivers/gpu/drm/drm_cache.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index f19d9acbe959..2c3fa5677f7e 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -112,8 +112,7 @@ drm_clflush_pages(struct page *pages[], unsigned long 
num_pages)
kunmap_atomic(page_virtual);
}
 #else
-   pr_err("Architecture has no drm_cache.c support\n");
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_pages);
@@ -143,8 +142,7 @@ drm_clflush_sg(struct sg_table *st)
if (wbinvd_on_all_cpus())
pr_err("Timed out waiting for cache flush\n");
 #else
-   pr_err("Architecture has no drm_cache.c support\n");
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_sg);
@@ -177,8 +175,7 @@ drm_clflush_virt_range(void *addr, unsigned long length)
if (wbinvd_on_all_cpus())
pr_err("Timed out waiting for cache flush\n");
 #else
-   pr_err("Architecture has no drm_cache.c support\n");
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_virt_range);
-- 
2.35.0



[Intel-gfx] [PATCH v3 0/3] drm/i915: Fix header test and log spam on !x86

2022-01-31 Thread Lucas De Marchi
Some minor fixes and changes to help porting i915 to arm64, or even
anything !x86.

v3: No changes, just submit to the right mailing list.

Lucas De Marchi (3):
  drm: Stop spamming log with drm_cache message
  drm/i915: Fix header test for !CONFIG_X86
  drm/i915: Do not spam log with missing arch support

 drivers/gpu/drm/drm_cache.c| 9 +++--
 drivers/gpu/drm/i915/i915_mm.h | 4 ++--
 2 files changed, 5 insertions(+), 8 deletions(-)

-- 
2.35.0



[Intel-gfx] [PATCH v3 2/3] drm/i915: Fix header test for !CONFIG_X86

2022-01-31 Thread Lucas De Marchi
Architectures others than x86 have a stub implementation calling
WARN_ON_ONCE(). The appropriate headers need to be included, otherwise
the header-test target will fail with:

  HDRTEST drivers/gpu/drm/i915/i915_mm.h
In file included from :
./drivers/gpu/drm/i915/i915_mm.h: In function ‘remap_io_mapping’:
./drivers/gpu/drm/i915/i915_mm.h:26:2: error: implicit declaration of function 
‘WARN_ON_ONCE’ [-Werror=implicit-function-declaration]
   26 |  WARN_ON_ONCE(1);
  |  ^~~~

v2: Do not include  since call to pr_err() has been
removed

Fixes: 67c430bbaae1 ("drm/i915: Skip remap_io_mapping() for non-x86 platforms")
Cc: Siva Mullati 
Signed-off-by: Lucas De Marchi 
---

v3: No changes from previous version, just submitting to the right
mailing list

 drivers/gpu/drm/i915/i915_mm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
index 76f1d53bdf34..3ad22bbe80eb 100644
--- a/drivers/gpu/drm/i915/i915_mm.h
+++ b/drivers/gpu/drm/i915/i915_mm.h
@@ -6,6 +6,7 @@
 #ifndef __I915_MM_H__
 #define __I915_MM_H__
 
+#include 
 #include 
 
 struct vm_area_struct;
-- 
2.35.0



[Intel-gfx] [PATCH v3 3/3] drm/i915: Do not spam log with missing arch support

2022-01-31 Thread Lucas De Marchi
Following what was done in drm_cache.c, when the stub for
remap_io_mapping() was added in commit 67c430bbaae1 ("drm/i915: Skip
remap_io_mapping() for non-x86 platforms"), it included a log message
with pr_err().  However just the warning is already enough and switching
to WARN_ONCE() allows us to keep the log message while avoiding log
spam.

Signed-off-by: Lucas De Marchi 
---

v3: No changes from previous version, just submitting to the right
mailing list

 drivers/gpu/drm/i915/i915_mm.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_mm.h b/drivers/gpu/drm/i915/i915_mm.h
index 3ad22bbe80eb..04c8974d822b 100644
--- a/drivers/gpu/drm/i915/i915_mm.h
+++ b/drivers/gpu/drm/i915/i915_mm.h
@@ -23,8 +23,7 @@ int remap_io_mapping(struct vm_area_struct *vma,
 unsigned long addr, unsigned long pfn, unsigned long size,
 struct io_mapping *iomap)
 {
-   pr_err("Architecture has no %s() and shouldn't be calling this 
function\n", __func__);
-   WARN_ON_ONCE(1);
+   WARN_ONCE(1, "Architecture has no drm_cache.c support\n");
return 0;
 }
 #endif
-- 
2.35.0



Re: [Intel-gfx] [PATCH 07/20] drm/i915/buddy: track available visible size

2022-01-31 Thread Thomas Hellström



On 1/26/22 16:21, Matthew Auld wrote:

Track the total amount of available visible memory, and also track
per-resource the amount of used visible memory. For now this is useful
for our debug output, and deciding if it is even worth calling into the
buddy allocator. In the future tracking the per-resource visible usage
will be useful for when deciding if we should attempt to evict certain
buffers.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 
---
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 55 ++-
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.h |  8 ++-
  drivers/gpu/drm/i915/intel_region_ttm.c   |  1 +
  3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c 
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index 53eb100688a6..6e5842155898 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -19,6 +19,8 @@ struct i915_ttm_buddy_manager {
struct drm_buddy mm;
struct list_head reserved;
struct mutex lock;
+   unsigned long visible_size;
+   unsigned long visible_avail;
u64 default_page_size;
  };
  
@@ -87,6 +89,13 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,

n_pages = size >> ilog2(mm->chunk_size);
  
  	mutex_lock(&bman->lock);

+   if (place->lpfn && place->lpfn <= bman->visible_size &&
+   n_pages > bman->visible_avail) {
+   mutex_unlock(&bman->lock);
+   err = -ENOSPC;
+   goto err_free_res;
+   }
+
err = drm_buddy_alloc_blocks(mm, (u64)place->fpfn << PAGE_SHIFT,
(u64)lpfn << PAGE_SHIFT,
(u64)n_pages << PAGE_SHIFT,
@@ -107,6 +116,30 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
mutex_unlock(&bman->lock);
}
  
+	if (place->lpfn && place->lpfn <= bman->visible_size) {

+   bman_res->used_visible_size = bman_res->base.num_pages;
+   } else {
+   struct drm_buddy_block *block;
+
+   list_for_each_entry(block, &bman_res->blocks, link) {
+   unsigned long start =
+   drm_buddy_block_offset(block) >> PAGE_SHIFT;
+   unsigned long end = start +
+   (drm_buddy_block_size(mm, block) >> PAGE_SHIFT);
+


Move this inside the if statement below? Or perhaps the compiler is 
smart enough to figure that out.




+   if (start < bman->visible_size) {
+   bman_res->used_visible_size +=
+   min(end, bman->visible_size) - start;
+   }
+   }
+   }


Reviewed-by: Thomas Hellstrom 




Re: [Intel-gfx] [PATCH 04/19] drm/i915: Move the power domain->well mappings to intel_display_power_map.c

2022-01-31 Thread Imre Deak
On Mon, Jan 31, 2022 at 02:15:25PM +0200, Jani Nikula wrote:
> On Fri, 28 Jan 2022, Imre Deak  wrote:
> > Move the list of platform specific power domain -> power well
> > definitions to intel_display_power_map.c. While at it group the
> > platforms' power domain macros with the corresponding power well lists
> > and keep all the power domain lists in the same order (matching the enum
> > order).
> >
> > No functional changes.
> >
> > Signed-off-by: Imre Deak 
> 
> The commit message should explain the why.
> 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h 
> > b/drivers/gpu/drm/i915/display/intel_display_power.h
> > index b30e6133c66d0..a0e68ae691021 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > @@ -197,6 +197,7 @@ struct intel_display_power_domain_set {
> > for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \
> > for_each_if(BIT_ULL(domain) & (mask))
> >  
> > +/* intel_display_power.c */
> >  int intel_power_domains_init(struct drm_i915_private *dev_priv);
> >  void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
> >  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool 
> > resume);
> > @@ -316,4 +317,8 @@ void chv_phy_powergate_lanes(struct intel_encoder 
> > *encoder,
> >  bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy 
> > phy,
> >   enum dpio_channel ch, bool override);
> >  
> > +/* intel_display_power_map.c */
> > +const char *
> > +intel_display_power_domain_str(enum intel_display_power_domain domain);
> > +
> >  #endif /* __INTEL_DISPLAY_POWER_H__ */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_internal.h 
> > b/drivers/gpu/drm/i915/display/intel_display_power_internal.h
> > new file mode 100644
> > index 0..3fc7c7d0bc9e9
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power_internal.h
> > @@ -0,0 +1,93 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2022 Intel Corporation
> > + */
> > +
> > +#ifndef __INTEL_DISPLAY_POWER_INTERNAL_H__
> > +#define __INTEL_DISPLAY_POWER_INTERNAL_H__
> > +
> > +#include "i915_reg_defs.h"
> > +
> > +#include "intel_display.h"
> > +#include "intel_display_power.h"
> > +
> > +struct i915_power_well_regs;
> > +
> > +/* Power well structure for haswell */
> > +struct i915_power_well_desc {
> > +   const char *name;
> > +   bool always_on;
> > +   u64 domains;
> > +   /* unique identifier for this power well */
> > +   enum i915_power_well_id id;
> > +   /*
> > +* Arbitraty data associated with this power well. Platform and power
> > +* well specific.
> > +*/
> > +   union {
> > +   struct {
> > +   /*
> > +* request/status flag index in the PUNIT power well
> > +* control/status registers.
> > +*/
> > +   u8 idx;
> > +   } vlv;
> > +   struct {
> > +   enum dpio_phy phy;
> > +   } bxt;
> > +   struct {
> > +   /*
> > +* request/status flag index in the power well
> > +* constrol/status registers.
> > +*/
> > +   u8 idx;
> > +   /* Mask of pipes whose IRQ logic is backed by the pw */
> > +   u8 irq_pipe_mask;
> > +   /*
> > +* Instead of waiting for the status bit to ack enables,
> > +* just wait a specific amount of time and then consider
> > +* the well enabled.
> > +*/
> > +   u16 fixed_enable_delay;
> > +   /* The pw is backing the VGA functionality */
> > +   bool has_vga:1;
> > +   bool has_fuses:1;
> > +   /*
> > +* The pw is for an ICL+ TypeC PHY port in
> > +* Thunderbolt mode.
> > +*/
> > +   bool is_tc_tbt:1;
> > +   } hsw;
> > +   };
> > +   const struct i915_power_well_ops *ops;
> > +};
> > +
> > +struct i915_power_well {
> > +   const struct i915_power_well_desc *desc;
> > +   /* power well enable/disable usage count */
> > +   int count;
> > +   /* cached hw enabled state */
> > +   bool hw_enabled;
> > +};
> > +
> > +/* intel_display_power.c */
> 
> I've put a lot of effort into splitting our (display) codebase towards
> having a 1-to-1 mapping between .c and .h files. This patch adds an odd
> split between two headers and two compilation units, and I don't think
> it's pretty.

This header includes struct definitions used by intel_display_power.c
and intel_display_power_map.c. I don't see why this would be a problem,
there are many other cases where multiple .c files include a header file
for the same reason.

> > +extern co

Re: [Intel-gfx] [PATCH 06/20] drm/i915: add I915_BO_ALLOC_TOPDOWN

2022-01-31 Thread Matthew Auld

On 31/01/2022 15:28, Thomas Hellström wrote:

On 1/26/22 16:21, Matthew Auld wrote:

If the user doesn't require CPU access for the buffer, then
ALLOC_TOPDOWN should be used, in order to prioritise allocating in the
non-mappable portion of LMEM.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 


I was wondering how this would work best with user-space not supplying 
any hints. Thinking that mappable LMEM buffers would be a minority, 
wouldn't it be better to have TOPDOWN behaviour set by default. It would 
then be migrated to mappable only if needed. And if the first usage is a 
cpu-map it would either be mapped in system or immediately migrated from 
pageless to mappable LMEM?


At this stage of the series I was mostly concerned with kernel internal 
users(including all of the selftests), for which pretty much all 
existing users want CPU access, so having that as the default seemed 
reasonable, and avoids needing to annotate lots of places with 
NEEDS_CPU_ACCESS. The TOPDOWN behaviour becomes the default for normal 
userspace objects later in the series, which only requires annotating 
one place.






---
  drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 15 +++
  drivers/gpu/drm/i915/gem/i915_gem_pages.c    |  3 +++
  drivers/gpu/drm/i915/gem/i915_gem_region.c   |  5 +
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 13 ++---
  drivers/gpu/drm/i915/gt/intel_gt.c   |  4 +++-
  drivers/gpu/drm/i915/i915_vma.c  |  3 +++
  drivers/gpu/drm/i915/intel_region_ttm.c  | 11 ---
  drivers/gpu/drm/i915/selftests/mock_region.c |  7 +--
  8 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h

index 71e778ecaeb8..29285aaf0477 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -319,15 +319,22 @@ struct drm_i915_gem_object {
  #define I915_BO_ALLOC_PM_VOLATILE BIT(4)
  /* Object needs to be restored early using memcpy during resume */
  #define I915_BO_ALLOC_PM_EARLY    BIT(5)
+/*
+ * Object is likely never accessed by the CPU. This will prioritise 
the BO to be
+ * allocated in the non-mappable portion of lmem. This is merely a 
hint, and if
+ * dealing with userspace objects the CPU fault handler is free to 
ignore this.

+ */
+#define I915_BO_ALLOC_TOPDOWN  BIT(6)
  #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
   I915_BO_ALLOC_VOLATILE | \
   I915_BO_ALLOC_CPU_CLEAR | \
   I915_BO_ALLOC_USER | \
   I915_BO_ALLOC_PM_VOLATILE | \
- I915_BO_ALLOC_PM_EARLY)
-#define I915_BO_READONLY  BIT(6)
-#define I915_TILING_QUIRK_BIT 7 /* unknown swizzling; do not 
release! */

-#define I915_BO_PROTECTED BIT(8)
+ I915_BO_ALLOC_PM_EARLY | \
+ I915_BO_ALLOC_TOPDOWN)
+#define I915_BO_READONLY  BIT(7)
+#define I915_TILING_QUIRK_BIT 8 /* unknown swizzling; do not 
release! */

+#define I915_BO_PROTECTED BIT(9)
  /**
   * @mem_flags - Mutable placement-related flags
   *
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c

index 7d2211fbe548..a95b4d72619f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -346,6 +346,9 @@ void *i915_gem_object_pin_map(struct 
drm_i915_gem_object *obj,

  !i915_gem_object_has_iomem(obj))
  return ERR_PTR(-ENXIO);
+    if (WARN_ON_ONCE(obj->flags & I915_BO_ALLOC_TOPDOWN))
+    return ERR_PTR(-EINVAL);
+
  assert_object_held(obj);
  pinned = !(type & I915_MAP_OVERRIDE);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c 
b/drivers/gpu/drm/i915/gem/i915_gem_region.c

index a4350227e9ae..f91e5a9c759d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -45,6 +45,11 @@ i915_gem_object_create_region(struct 
intel_memory_region *mem,

  GEM_BUG_ON(flags & ~I915_BO_ALLOC_FLAGS);
+    if (WARN_ON_ONCE(flags & I915_BO_ALLOC_TOPDOWN &&
+ (flags & I915_BO_ALLOC_CPU_CLEAR ||
+  flags & I915_BO_ALLOC_PM_EARLY)))
+    return ERR_PTR(-EINVAL);
+
  if (!mem)
  return ERR_PTR(-ENODEV);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c

index d9a04c7d41b1..e60b677ecd54 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -127,10 +127,14 @@ i915_ttm_place_from_region(const struct 
intel_memory_region *mr,

  place->mem_type = intel_region_to_ttm_type(mr);
  if (flags & I915_BO_ALLOC_CONTIGUOUS)
-    place->flags = TTM_PL_FLAG_CONTIGUOUS;
+    place->flags |= TTM_PL_FLAG_CONTIGUOUS;
  if (mr->io_size && mr->io_size < mr->total) {
- 

Re: [Intel-gfx] [PATCH 06/20] drm/i915: add I915_BO_ALLOC_TOPDOWN

2022-01-31 Thread Thomas Hellström

On 1/26/22 16:21, Matthew Auld wrote:

If the user doesn't require CPU access for the buffer, then
ALLOC_TOPDOWN should be used, in order to prioritise allocating in the
non-mappable portion of LMEM.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 


I was wondering how this would work best with user-space not supplying 
any hints. Thinking that mappable LMEM buffers would be a minority, 
wouldn't it be better to have TOPDOWN behaviour set by default. It would 
then be migrated to mappable only if needed. And if the first usage is a 
cpu-map it would either be mapped in system or immediately migrated from 
pageless to mappable LMEM?




---
  drivers/gpu/drm/i915/gem/i915_gem_object_types.h | 15 +++
  drivers/gpu/drm/i915/gem/i915_gem_pages.c|  3 +++
  drivers/gpu/drm/i915/gem/i915_gem_region.c   |  5 +
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 13 ++---
  drivers/gpu/drm/i915/gt/intel_gt.c   |  4 +++-
  drivers/gpu/drm/i915/i915_vma.c  |  3 +++
  drivers/gpu/drm/i915/intel_region_ttm.c  | 11 ---
  drivers/gpu/drm/i915/selftests/mock_region.c |  7 +--
  8 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 71e778ecaeb8..29285aaf0477 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -319,15 +319,22 @@ struct drm_i915_gem_object {
  #define I915_BO_ALLOC_PM_VOLATILE BIT(4)
  /* Object needs to be restored early using memcpy during resume */
  #define I915_BO_ALLOC_PM_EARLYBIT(5)
+/*
+ * Object is likely never accessed by the CPU. This will prioritise the BO to 
be
+ * allocated in the non-mappable portion of lmem. This is merely a hint, and if
+ * dealing with userspace objects the CPU fault handler is free to ignore this.
+ */
+#define I915_BO_ALLOC_TOPDOWNBIT(6)
  #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
 I915_BO_ALLOC_VOLATILE | \
 I915_BO_ALLOC_CPU_CLEAR | \
 I915_BO_ALLOC_USER | \
 I915_BO_ALLOC_PM_VOLATILE | \
-I915_BO_ALLOC_PM_EARLY)
-#define I915_BO_READONLY  BIT(6)
-#define I915_TILING_QUIRK_BIT 7 /* unknown swizzling; do not release! */
-#define I915_BO_PROTECTED BIT(8)
+I915_BO_ALLOC_PM_EARLY | \
+I915_BO_ALLOC_TOPDOWN)
+#define I915_BO_READONLY  BIT(7)
+#define I915_TILING_QUIRK_BIT 8 /* unknown swizzling; do not release! */
+#define I915_BO_PROTECTED BIT(9)
/**
 * @mem_flags - Mutable placement-related flags
 *
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 7d2211fbe548..a95b4d72619f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -346,6 +346,9 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object 
*obj,
!i915_gem_object_has_iomem(obj))
return ERR_PTR(-ENXIO);
  
+	if (WARN_ON_ONCE(obj->flags & I915_BO_ALLOC_TOPDOWN))

+   return ERR_PTR(-EINVAL);
+
assert_object_held(obj);
  
  	pinned = !(type & I915_MAP_OVERRIDE);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c 
b/drivers/gpu/drm/i915/gem/i915_gem_region.c
index a4350227e9ae..f91e5a9c759d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -45,6 +45,11 @@ i915_gem_object_create_region(struct intel_memory_region 
*mem,
  
  	GEM_BUG_ON(flags & ~I915_BO_ALLOC_FLAGS);
  
+	if (WARN_ON_ONCE(flags & I915_BO_ALLOC_TOPDOWN &&

+(flags & I915_BO_ALLOC_CPU_CLEAR ||
+ flags & I915_BO_ALLOC_PM_EARLY)))
+   return ERR_PTR(-EINVAL);
+
if (!mem)
return ERR_PTR(-ENODEV);
  
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c

index d9a04c7d41b1..e60b677ecd54 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -127,10 +127,14 @@ i915_ttm_place_from_region(const struct 
intel_memory_region *mr,
place->mem_type = intel_region_to_ttm_type(mr);
  
  	if (flags & I915_BO_ALLOC_CONTIGUOUS)

-   place->flags = TTM_PL_FLAG_CONTIGUOUS;
+   place->flags |= TTM_PL_FLAG_CONTIGUOUS;
if (mr->io_size && mr->io_size < mr->total) {
-   place->fpfn = 0;
-   place->lpfn = mr->io_size >> PAGE_SHIFT;
+   if (flags & I915_BO_ALLOC_TOPDOWN) {
+   place->flags |= TTM_PL_FLAG_TOPDOWN;
+   } else {
+   place->fpfn = 0;
+   place->lpfn = mr->io_size >> PAGE_SHIFT;
+ 

Re: [Intel-gfx] [PATCH 04/20] drm/i915: add io_size plumbing

2022-01-31 Thread Thomas Hellström



On 1/26/22 16:21, Matthew Auld wrote:

With small LMEM-BAR we need to be able to differentiate between the
total size of LMEM, and how much of it is CPU mappable. The end goal is
to be able to utilize the entire range, even if part of is it not CPU
accessible.

Signed-off-by: Matthew Auld 
Cc: Thomas Hellström 


Reviewed-by: Thomas Hellström 



---
  drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 2 +-
  drivers/gpu/drm/i915/gem/i915_gem_stolen.c   | 8 +---
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c  | 2 +-
  drivers/gpu/drm/i915/gem/selftests/huge_pages.c  | 2 +-
  drivers/gpu/drm/i915/gt/intel_region_lmem.c  | 6 +-
  drivers/gpu/drm/i915/intel_memory_region.c   | 6 +-
  drivers/gpu/drm/i915/intel_memory_region.h   | 2 ++
  drivers/gpu/drm/i915/selftests/intel_memory_region.c | 8 
  drivers/gpu/drm/i915/selftests/mock_region.c | 6 --
  drivers/gpu/drm/i915/selftests/mock_region.h | 3 ++-
  10 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 6c57b0a79c8a..a9aca11cedbb 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -696,7 +696,7 @@ struct intel_memory_region *i915_gem_shmem_setup(struct 
drm_i915_private *i915,
  {
return intel_memory_region_create(i915, 0,
  totalram_pages() << PAGE_SHIFT,
- PAGE_SIZE, 0,
+ PAGE_SIZE, 0, 0,
  type, instance,
  &shmem_region_ops);
  }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c 
b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 26975d857776..387b48686851 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -490,6 +490,7 @@ static int i915_gem_init_stolen(struct intel_memory_region 
*mem)
  
  	/* Exclude the reserved region from driver use */

mem->region.end = reserved_base - 1;
+   mem->io_size = resource_size(&mem->region);
  
  	/* It is possible for the reserved area to end before the end of stolen

 * memory, so just consider the start. */
@@ -746,7 +747,7 @@ static int init_stolen_lmem(struct intel_memory_region *mem)
  
  	if (!io_mapping_init_wc(&mem->iomap,

mem->io_start,
-   resource_size(&mem->region)))
+   mem->io_size))
return -EIO;
  
  	/*

@@ -801,7 +802,8 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, 
u16 type,
I915_GTT_PAGE_SIZE_4K;
  
  	mem = intel_memory_region_create(i915, lmem_base, lmem_size,

-min_page_size, io_start,
+min_page_size,
+io_start, lmem_size,
 type, instance,
 &i915_region_stolen_lmem_ops);
if (IS_ERR(mem))
@@ -832,7 +834,7 @@ i915_gem_stolen_smem_setup(struct drm_i915_private *i915, 
u16 type,
mem = intel_memory_region_create(i915,
 intel_graphics_stolen_res.start,
 
resource_size(&intel_graphics_stolen_res),
-PAGE_SIZE, 0, type, instance,
+PAGE_SIZE, 0, 0, type, instance,
 &i915_region_stolen_smem_ops);
if (IS_ERR(mem))
return mem;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 84cae740b4a5..e1140ca3d9a0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -1103,7 +1103,7 @@ i915_gem_ttm_system_setup(struct drm_i915_private *i915,
  
  	mr = intel_memory_region_create(i915, 0,

totalram_pages() << PAGE_SHIFT,
-   PAGE_SIZE, 0,
+   PAGE_SIZE, 0, 0,
type, instance,
&ttm_system_region_ops);
if (IS_ERR(mr))
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index f36191ebf964..42db9cd30978 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -499,7 +499,7 @@ static int igt_mock_memory_region_huge_pages(void *arg)
int bit;
int err = 0;
  
-	mem = mock_region_create(i915, 0, SZ_2G, I915_GTT_PAGE_SIZE_4K, 0);

+   mem = mock_region_create(i915, 0, SZ_2G

Re: [Intel-gfx] [PATCH v2 00/17] drm/i915: M/N cleanup

2022-01-31 Thread Jani Nikula
On Fri, 28 Jan 2022, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> Rehashed version of the M/N cleanup after Jani (rightly)
> complained about the legibility of some of the patches in
> the v1 series. These are chunked to a finer pulp, some got
> revised a bit, and I left out a few of the FDI related
> things for now. I'll revisit the PCH port/FDI topic later,
> for now I just slapped in an extra patch to make sure we
> don't try to use DRRS on PCH ports.

I've commented on one bug that needs fixing, and some nitpicks and
future suggestions, but overall the series is

Reviewed-by: Jani Nikula 

>
> Ville Syrjälä (17):
>   drm/i915: Nuke intel_dp_set_m_n()
>   drm/i915: Nuke intel_dp_get_m_n()
>   drm/i915: Nuke ilk_get_fdi_m_n_config()
>   drm/i915: Split intel_cpu_transcoder_set_m_n() into M1/N1 vs. M2/N2
> variants
>   drm/i915: Split intel_cpu_transcoder_get_m_n() into M1/N1 vs. M2/N2
> variants
>   drm/i915: Pass crtc+cpu_transcoder to intel_cpu_transcoder_set_m_n()
>   drm/i915: Move PCH transcoder M/N setup into the PCH code
>   drm/i915: Move M/N setup to a more logical place on ddi platforms
>   drm/i915: Extract {i9xx,ilk}_configure_cpu_transcoder()
>   drm/i915: Disable DRRS on IVB/HSW port != A
>   drm/i915: Extract can_enable_drrs()
>   drm/i915: Fix intel_cpu_transcoder_has_m2_n2()
>   drm/i915: Clear DP M2/N2 when not doing DRRS
>   drm/i915: Program pch transcoder m2/n2
>   drm/i915: Dump dp_m2_n2 always
>   drm/i915: Always check dp_m2_n2 on pre-bdw
>   drm/i915: Document BDW+ DRRS M/N programming requirements
>
>  drivers/gpu/drm/i915/display/g4x_dp.c |  18 +-
>  drivers/gpu/drm/i915/display/intel_ddi.c  |  14 +-
>  drivers/gpu/drm/i915/display/intel_display.c  | 266 --
>  drivers/gpu/drm/i915/display/intel_display.h  |  32 ++-
>  .../drm/i915/display/intel_display_types.h|  19 --
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |   2 -
>  drivers/gpu/drm/i915/display/intel_drrs.c |  50 +++-
>  .../gpu/drm/i915/display/intel_pch_display.c  |  54 +++-
>  .../gpu/drm/i915/display/intel_pch_display.h  |   6 +
>  9 files changed, 259 insertions(+), 202 deletions(-)

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH v2 12/17] drm/i915: Fix intel_cpu_transcoder_has_m2_n2()

2022-01-31 Thread Jani Nikula
On Fri, 28 Jan 2022, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> M2/N2 values are present for all ilk-ivb,vlv,chv (and hsw edp).
> Make the code reflect that.

Nitpick, it's not called intel_cpu_transcoder_has_m2_n2() until in the
next patch.

Side note, I've also been looking at this bit in intel_drrs_set_state():

if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv))
intel_drrs_set_refresh_rate_m_n(crtc_state, refresh_type);
else if (DISPLAY_VER(dev_priv) > 6)
intel_drrs_set_refresh_rate_pipeconf(crtc_state, refresh_type);

and wondering if that should be deduplicated with the
transcoder_has_m2_n2() somehow. This is all a bit confusing with the
slightly different conditions.

BR,
Jani.

>
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 1e97279ba268..67c7bbbe5c88 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3149,11 +3149,7 @@ static bool transcoder_has_m2_n2(struct 
> drm_i915_private *dev_priv,
>   if (IS_HASWELL(dev_priv))
>   return transcoder == TRANSCODER_EDP;
>  
> - /*
> -  * Strictly speaking some registers are available before
> -  * gen7, but we only support DRRS on gen7+
> -  */
> - return DISPLAY_VER(dev_priv) == 7 || IS_CHERRYVIEW(dev_priv);
> + return IS_DISPLAY_VER(dev_priv, 5, 7) || IS_CHERRYVIEW(dev_priv);
>  }
>  
>  void intel_cpu_transcoder_set_m1_n1(struct intel_crtc *crtc,

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH v5 1/5] drm/i915: add needs_compact_pt flag

2022-01-31 Thread Thomas Hellström



On 1/31/22 15:19, Robert Beckett wrote:



On 27/01/2022 09:37, Thomas Hellström (Intel) wrote:


On 1/26/22 18:11, Robert Beckett wrote:



On 26/01/2022 13:49, Thomas Hellström (Intel) wrote:


On 1/25/22 20:35, Robert Beckett wrote:

From: Ramalingam C 

Add a new platform flag, needs_compact_pt, to mark the requirement of
compact pt layout support for the ppGTT when using 64K GTT pages.

With this flag has_64k_pages will only indicate requirement of 64K
GTT page sizes or larger for device local memory access.

Suggested-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
---
  drivers/gpu/drm/i915/i915_drv.h  | 10 +++---
  drivers/gpu/drm/i915/i915_pci.c  |  2 ++
  drivers/gpu/drm/i915/intel_device_info.h |  1 +
  3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h 
b/drivers/gpu/drm/i915/i915_drv.h

index 44c1f98144b4..1258b7779705 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1512,12 +1512,16 @@ IS_SUBPLATFORM(const struct 
drm_i915_private *i915,

  /*
   * Set this flag, when platform requires 64K GTT page sizes or 
larger for
- * device local memory access. Also this flag implies that we 
require or
- * at least support the compact PT layout for the ppGTT when 
using the 64K

- * GTT pages.


Why do we remove these comment lines?
Because HAS_64K_PAGES now means just 64K page, it no longer means 
also requires compact pt.
This is to support other products that will have 64K but not have 
the PDE non-sharing restriction in future.


Those lines moved to the next change NEEDS_COMPACT_PT, which is now 
separate.


Yes, NEEDS_COMPACT_PT indicates that compact is *required* but does 
"HAS_64K_PAGES" still mean compact is supported? That information is 
lost.

Not any more.
I discussed the ambiguity of the original wording with mauld on irc.
We came to the conclusion that HAS_64K_PAGES should mean just that, 
that the hw has support for 64K pages, and says nothing about 
compact-pt at all.


NEEDS_COMPACT_PT means that the hw has compact-pt support and the 
driver is required to use it as it is a hw limitation.


There will be other devices that can support compact-pt but do not 
mandate its use. In this case, the current code would not use them, 
but there is potential for some future opportunistic use of that in 
the driver if desired (currently expected to include accelerated 
move/clear). If any opportunistic use is added to the driver, a new 
flag can be added along with the code that uses it to indicate 
compact-pt availability that is not mandatory (HAS_COMPACT_PT most 
likely), but as there is no code requiring it currently it should not 
be added yet, and the comments left as this patch does.



Ok, Thanks for the clarification.

Reviewed-by: Thomas Hellström 





/Thomas




Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/: Re-work clflush_write32

2022-01-31 Thread Tvrtko Ursulin



On 28/01/2022 22:10, Michael Cheng wrote:

Use drm_clflush_virt_range instead of clflushopt and remove the memory
barrier, since drm_clflush_virt_range takes care of that.

Signed-off-by: Michael Cheng 
---
  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 498b458fd784..0854276ff7ba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1332,10 +1332,8 @@ static void *reloc_vaddr(struct i915_vma *vma,
  static void clflush_write32(u32 *addr, u32 value, unsigned int flushes)
  {
if (unlikely(flushes & (CLFLUSH_BEFORE | CLFLUSH_AFTER))) {
-   if (flushes & CLFLUSH_BEFORE) {
-   clflushopt(addr);
-   mb();
-   }
+   if (flushes & CLFLUSH_BEFORE)
+   drm_clflush_virt_range(addr, sizeof(addr));
  
  		*addr = value;
  
@@ -1347,7 +1345,7 @@ static void clflush_write32(u32 *addr, u32 value, unsigned int flushes)

 * to ensure ordering of clflush wrt to the system.
 */
if (flushes & CLFLUSH_AFTER)
-   clflushopt(addr);
+   drm_clflush_virt_range(addr, sizeof(addr));
} else
*addr = value;
  }


Slightly annoying thing here (maybe in some other patches from the series as 
well) is that the change adds a function call to x86 only code path, because 
relocations are not supported on discrete as per:

static in
eb_validate_vma(...)
/* Relocations are disallowed for all platforms after TGL-LP.  This
 * also covers all platforms with local memory.
 */

if (entry->relocation_count &&
GRAPHICS_VER(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915))
return -EINVAL;

How acceptable would be, for the whole series, to introduce a static inline 
i915 cluflush wrapper and so be able to avoid functions calls on x86? Is this 
something that has been discussed and discounted already?

Regards,

Tvrtko

P.S. Hmm I am now reminded of my really old per platform build patches. With 
them you would be able to compile out large portions of the driver when 
building for ARM. Probably like a 3rd if my memory serves me right.


Re: [Intel-gfx] [PATCH v2 06/17] drm/i915: Pass crtc+cpu_transcoder to intel_cpu_transcoder_set_m_n()

2022-01-31 Thread Jani Nikula
On Fri, 28 Jan 2022, Ville Syrjala  wrote:
> From: Ville Syrjälä 
>
> Instead of passing in the whole crtc state let's pass in just
> the bits of state we need. This will help with the DRRS code
> which shouldn't really be accessing the atomic state stuff directly
> as it gets called outside the normal atomic flows.

Overall looks good, one bug crept in.

>
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |  6 ++--
>  drivers/gpu/drm/i915/display/intel_display.c | 37 ++--
>  drivers/gpu/drm/i915/display/intel_display.h |  6 ++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  |  3 +-
>  drivers/gpu/drm/i915/display/intel_drrs.c|  5 ++-
>  5 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b02b327331f8..360f62665b54 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2498,6 +2498,8 @@ static void intel_ddi_pre_enable_dp(struct 
> intel_atomic_state *state,
>   const struct drm_connector_state 
> *conn_state)
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
>   if (DISPLAY_VER(dev_priv) >= 12)
>   tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
> @@ -2510,9 +2512,9 @@ static void intel_ddi_pre_enable_dp(struct 
> intel_atomic_state *state,
>   if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) {
>   intel_ddi_set_dp_msa(crtc_state, conn_state);
>  
> - intel_cpu_transcoder_set_m1_n1(crtc_state,
> + intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
>  &crtc_state->dp_m_n);
> - intel_cpu_transcoder_set_m2_n2(crtc_state,
> + intel_cpu_transcoder_set_m2_n2(crtc, cpu_transcoder,
>  &crtc_state->dp_m2_n2);
>   }
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 79f22a3f2e20..0392803bb790 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -118,7 +118,7 @@
>  
>  static void intel_set_transcoder_timings(const struct intel_crtc_state 
> *crtc_state);
>  static void intel_set_pipe_src_size(const struct intel_crtc_state 
> *crtc_state);
> -static void intel_pch_transcoder_set_m_n(const struct intel_crtc_state 
> *crtc_state,
> +static void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
>const struct intel_link_m_n *m_n);
>  static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state);
>  static void ilk_set_pipeconf(const struct intel_crtc_state *crtc_state);
> @@ -1816,6 +1816,7 @@ static void ilk_crtc_enable(struct intel_atomic_state 
> *state,
>   const struct intel_crtc_state *new_crtc_state =
>   intel_atomic_get_new_crtc_state(state, crtc);
>   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
>   enum pipe pipe = crtc->pipe;
>  
>   if (drm_WARN_ON(&dev_priv->drm, crtc->active))
> @@ -1836,12 +1837,11 @@ static void ilk_crtc_enable(struct intel_atomic_state 
> *state,
>  
>   if (intel_crtc_has_dp_encoder(new_crtc_state)) {
>   if (new_crtc_state->has_pch_encoder) {
> - intel_pch_transcoder_set_m_n(new_crtc_state,
> -  &new_crtc_state->dp_m_n);
> + intel_pch_transcoder_set_m_n(crtc, 
> &new_crtc_state->dp_m_n);
>   } else {
> - intel_cpu_transcoder_set_m1_n1(new_crtc_state,
> + intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
>  &new_crtc_state->dp_m_n);
> - intel_cpu_transcoder_set_m2_n2(new_crtc_state,
> + intel_cpu_transcoder_set_m2_n2(crtc, cpu_transcoder,
>  
> &new_crtc_state->dp_m2_n2);
>   }
>   }
> @@ -1850,7 +1850,7 @@ static void ilk_crtc_enable(struct intel_atomic_state 
> *state,
>   intel_set_pipe_src_size(new_crtc_state);
>  
>   if (new_crtc_state->has_pch_encoder)
> - intel_cpu_transcoder_set_m1_n1(new_crtc_state,
> + intel_cpu_transcoder_set_m1_n1(crtc, cpu_transcoder,
>  &new_crtc_state->fdi_m_n);
>  
>   ilk_set_pipeconf(new_crtc_state);
> @@ -2017,7 +2017,7 @@ static void hsw_configure_cpu_transcoder(const struct 
> intel_crtc_state *crtc_sta
>  cr

Re: [Intel-gfx] [PATCH v5 1/5] drm/i915: add needs_compact_pt flag

2022-01-31 Thread Robert Beckett




On 27/01/2022 09:37, Thomas Hellström (Intel) wrote:


On 1/26/22 18:11, Robert Beckett wrote:



On 26/01/2022 13:49, Thomas Hellström (Intel) wrote:


On 1/25/22 20:35, Robert Beckett wrote:

From: Ramalingam C 

Add a new platform flag, needs_compact_pt, to mark the requirement of
compact pt layout support for the ppGTT when using 64K GTT pages.

With this flag has_64k_pages will only indicate requirement of 64K
GTT page sizes or larger for device local memory access.

Suggested-by: Matthew Auld 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
---
  drivers/gpu/drm/i915/i915_drv.h  | 10 +++---
  drivers/gpu/drm/i915/i915_pci.c  |  2 ++
  drivers/gpu/drm/i915/intel_device_info.h |  1 +
  3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h 
b/drivers/gpu/drm/i915/i915_drv.h

index 44c1f98144b4..1258b7779705 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1512,12 +1512,16 @@ IS_SUBPLATFORM(const struct drm_i915_private 
*i915,

  /*
   * Set this flag, when platform requires 64K GTT page sizes or 
larger for
- * device local memory access. Also this flag implies that we 
require or
- * at least support the compact PT layout for the ppGTT when using 
the 64K

- * GTT pages.


Why do we remove these comment lines?
Because HAS_64K_PAGES now means just 64K page, it no longer means also 
requires compact pt.
This is to support other products that will have 64K but not have the 
PDE non-sharing restriction in future.


Those lines moved to the next change NEEDS_COMPACT_PT, which is now 
separate.


Yes, NEEDS_COMPACT_PT indicates that compact is *required* but does 
"HAS_64K_PAGES" still mean compact is supported? That information is lost.

Not any more.
I discussed the ambiguity of the original wording with mauld on irc.
We came to the conclusion that HAS_64K_PAGES should mean just that, that 
the hw has support for 64K pages, and says nothing about compact-pt at all.


NEEDS_COMPACT_PT means that the hw has compact-pt support and the driver 
is required to use it as it is a hw limitation.


There will be other devices that can support compact-pt but do not 
mandate its use. In this case, the current code would not use them, but 
there is potential for some future opportunistic use of that in the 
driver if desired (currently expected to include accelerated 
move/clear). If any opportunistic use is added to the driver, a new flag 
can be added along with the code that uses it to indicate compact-pt 
availability that is not mandatory (HAS_COMPACT_PT most likely), but as 
there is no code requiring it currently it should not be added yet, and 
the comments left as this patch does.




/Thomas




Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/gt: Re-work invalidate_csb_entries

2022-01-31 Thread Mika Kuoppala
Tvrtko Ursulin  writes:

> On 28/01/2022 22:10, Michael Cheng wrote:
>> Re-work invalidate_csb_entries to use drm_clflush_virt_range. This will
>> prevent compiler errors when building for non-x86 architectures.
>> 
>> Signed-off-by: Michael Cheng 
>> ---
>>   drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c 
>> b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
>> index 960a9aaf4f3a..90b5daf9433d 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
>> @@ -1647,8 +1647,8 @@ cancel_port_requests(struct intel_engine_execlists * 
>> const execlists,
>>   
>>   static void invalidate_csb_entries(const u64 *first, const u64 *last)
>>   {
>> -clflush((void *)first);
>> -clflush((void *)last);
>> +drm_clflush_virt_range((void *)first, sizeof(*first));
>> +drm_clflush_virt_range((void *)last, sizeof(*last));
>
> How about dropping the helper and from the single call site do:
>
> drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0]));
>
> One less function call and CSB is a single cacheline before Gen11 ayway, 
> two afterwards, so overall better conversion I think. How does that sound?

It would definitely work. Now trying to remember why it went into
explicit clflushes: iirc as this is gpu/cpu coherency, the
wbinvd_on_all_cpus we get with *virt_range would then be just
unnecessary perf hit.

-Mika

>
> Regards,
>
> Tvrtko
>
>>   }
>>   
>>   /*
>> 


Re: [Intel-gfx] [PATCH v2 2/4] drm/i915/gt: Re-work invalidate_csb_entries

2022-01-31 Thread Tvrtko Ursulin



On 28/01/2022 22:10, Michael Cheng wrote:

Re-work invalidate_csb_entries to use drm_clflush_virt_range. This will
prevent compiler errors when building for non-x86 architectures.

Signed-off-by: Michael Cheng 
---
  drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c 
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index 960a9aaf4f3a..90b5daf9433d 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -1647,8 +1647,8 @@ cancel_port_requests(struct intel_engine_execlists * 
const execlists,
  
  static void invalidate_csb_entries(const u64 *first, const u64 *last)

  {
-   clflush((void *)first);
-   clflush((void *)last);
+   drm_clflush_virt_range((void *)first, sizeof(*first));
+   drm_clflush_virt_range((void *)last, sizeof(*last));


How about dropping the helper and from the single call site do:

drm_clflush_virt_range(&buf[0], num_entries * sizeof(buf[0]));

One less function call and CSB is a single cacheline before Gen11 ayway, 
two afterwards, so overall better conversion I think. How does that sound?


Regards,

Tvrtko


  }
  
  /*




Re: [Intel-gfx] [PATCH - v3] drm/i915: Discard large BIOS framebuffers causing display corruption.

2022-01-31 Thread Ashish Arora


> On 12-Jan-2022, at 7:07 PM, Ville Syrjälä  
> wrote:
> 
> On Tue, Jan 11, 2022 at 07:55:22AM +, Ashish Arora wrote:
>> From: Ashish Arora 
>> 
>> On certain 4k panels and Macs, the BIOS framebuffer is larger than what
>> panel requires causing display corruption. Introduce a check for the same.
> 
> If a larger fb causes corruption then there is a real bug somewhere.
Hi Ville

I and some members of the t2 linux group would like to have more details of 
this bug.
> 
>> 
>> 
>> Signed-off-by: Ashish Arora 
>> Reviewed-by: Aun-Ali Zaidi 
>> ---
>> V2 :- Use != instead of < and >
>> V3 :- Mention Macs (Thanks to Orlando)
>> drivers/gpu/drm/i915/display/intel_fbdev.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c 
>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> index 842c04e63..16b1c82b2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>> @@ -181,10 +181,10 @@ static int intelfb_create(struct drm_fb_helper *helper,
>>  int ret;
>> 
>>  if (intel_fb &&
>> -(sizes->fb_width > intel_fb->base.width ||
>> - sizes->fb_height > intel_fb->base.height)) {
>> +(sizes->fb_width != intel_fb->base.width ||
>> + sizes->fb_height != intel_fb->base.height)) {
>>  drm_dbg_kms(&dev_priv->drm,
>> -"BIOS fb too small (%dx%d), we require (%dx%d),"
>> +"BIOS fb not valid (%dx%d), we require (%dx%d),"
>>  " releasing it\n",
>>  intel_fb->base.width, intel_fb->base.height,
>>  sizes->fb_width, sizes->fb_height);
>> -- 
>> 2.25.1
>> 
> 
> -- 
> Ville Syrjälä
> Intel



Re: [Intel-gfx] [PATCH v2] drm/i915/display/vrr: Reset VRR capable property on a long hpd

2022-01-31 Thread Jani Nikula
On Wed, 26 Jan 2022, Manasi Navare  wrote:
> With some VRR panels, user can turn VRR ON/OFF on the fly from the panel 
> settings.
> When VRR is turned OFF ,sends a long HPD to the driver clearing the Ignore 
> MSA bit
> in the DPCD. Currently the driver parses that onevery HPD but fails to reset
> the corresponding VRR Capable Connector property.
> Hence the userspace still sees this as VRR Capable panel which is incorrect.
>
> Fix this by explicitly resetting the connector property.
>
> v2: Reset vrr capable if status == connector_disconnected
> v3: Use i915 and use bool vrr_capable (Jani Nikula)
> Cc: Jani Nikula 
> Signed-off-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4d4579a301f6..687cb37bb22a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4446,6 +4446,12 @@ intel_dp_detect(struct drm_connector *connector,
>   memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
>   memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
>  
> + /* Reset VRR Capable property */
> + drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s] VRR capable: 
> FALSE\n",
> + connector->base.id, connector->name);

Both the comment and the logging here seem excessive.

> + drm_connector_set_vrr_capable_property(connector,
> +false);
> +

Fits on one line.

>   if (intel_dp->is_mst) {
>   drm_dbg_kms(&dev_priv->drm,
>   "MST device may have disappeared %d vs 
> %d\n",
> @@ -4560,15 +4566,17 @@ static int intel_dp_get_modes(struct drm_connector 
> *connector)
>  {
>   struct intel_connector *intel_connector = to_intel_connector(connector);
>   struct edid *edid;
> + struct drm_i915_private *i915 = to_i915(connector->dev);
>   int num_modes = 0;
>  
>   edid = intel_connector->detect_edid;
>   if (edid) {
> - num_modes = intel_connector_update_modes(connector, edid);
> + bool vrr_capable = intel_vrr_is_capable(connector);
>  
> - if (intel_vrr_is_capable(connector))
> - drm_connector_set_vrr_capable_property(connector,
> -true);
> + num_modes = intel_connector_update_modes(connector, edid);

intel_vrr_is_capable() probably needs to happen after
intel_connector_update_modes(), right?

BR,
Jani.

> + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> + connector->base.id, connector->name, 
> yesno(vrr_capable));
> + drm_connector_set_vrr_capable_property(connector, vrr_capable);
>   }
>  
>   /* Also add fixed mode, which may or may not be present in EDID */

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH 04/19] drm/i915: Move the power domain->well mappings to intel_display_power_map.c

2022-01-31 Thread Jani Nikula
On Fri, 28 Jan 2022, Imre Deak  wrote:
> Move the list of platform specific power domain -> power well
> definitions to intel_display_power_map.c. While at it group the
> platforms' power domain macros with the corresponding power well lists
> and keep all the power domain lists in the same order (matching the enum
> order).
>
> No functional changes.
>
> Signed-off-by: Imre Deak 

The commit message should explain the why.

> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h 
> b/drivers/gpu/drm/i915/display/intel_display_power.h
> index b30e6133c66d0..a0e68ae691021 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -197,6 +197,7 @@ struct intel_display_power_domain_set {
>   for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \
>   for_each_if(BIT_ULL(domain) & (mask))
>  
> +/* intel_display_power.c */
>  int intel_power_domains_init(struct drm_i915_private *dev_priv);
>  void intel_power_domains_cleanup(struct drm_i915_private *dev_priv);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool 
> resume);
> @@ -316,4 +317,8 @@ void chv_phy_powergate_lanes(struct intel_encoder 
> *encoder,
>  bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy 
> phy,
> enum dpio_channel ch, bool override);
>  
> +/* intel_display_power_map.c */
> +const char *
> +intel_display_power_domain_str(enum intel_display_power_domain domain);
> +
>  #endif /* __INTEL_DISPLAY_POWER_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_internal.h 
> b/drivers/gpu/drm/i915/display/intel_display_power_internal.h
> new file mode 100644
> index 0..3fc7c7d0bc9e9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/display/intel_display_power_internal.h
> @@ -0,0 +1,93 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef __INTEL_DISPLAY_POWER_INTERNAL_H__
> +#define __INTEL_DISPLAY_POWER_INTERNAL_H__
> +
> +#include "i915_reg_defs.h"
> +
> +#include "intel_display.h"
> +#include "intel_display_power.h"
> +
> +struct i915_power_well_regs;
> +
> +/* Power well structure for haswell */
> +struct i915_power_well_desc {
> + const char *name;
> + bool always_on;
> + u64 domains;
> + /* unique identifier for this power well */
> + enum i915_power_well_id id;
> + /*
> +  * Arbitraty data associated with this power well. Platform and power
> +  * well specific.
> +  */
> + union {
> + struct {
> + /*
> +  * request/status flag index in the PUNIT power well
> +  * control/status registers.
> +  */
> + u8 idx;
> + } vlv;
> + struct {
> + enum dpio_phy phy;
> + } bxt;
> + struct {
> + /*
> +  * request/status flag index in the power well
> +  * constrol/status registers.
> +  */
> + u8 idx;
> + /* Mask of pipes whose IRQ logic is backed by the pw */
> + u8 irq_pipe_mask;
> + /*
> +  * Instead of waiting for the status bit to ack enables,
> +  * just wait a specific amount of time and then consider
> +  * the well enabled.
> +  */
> + u16 fixed_enable_delay;
> + /* The pw is backing the VGA functionality */
> + bool has_vga:1;
> + bool has_fuses:1;
> + /*
> +  * The pw is for an ICL+ TypeC PHY port in
> +  * Thunderbolt mode.
> +  */
> + bool is_tc_tbt:1;
> + } hsw;
> + };
> + const struct i915_power_well_ops *ops;
> +};
> +
> +struct i915_power_well {
> + const struct i915_power_well_desc *desc;
> + /* power well enable/disable usage count */
> + int count;
> + /* cached hw enabled state */
> + bool hw_enabled;
> +};
> +
> +/* intel_display_power.c */

I've put a lot of effort into splitting our (display) codebase towards
having a 1-to-1 mapping between .c and .h files. This patch adds an odd
split between two headers and two compilation units, and I don't think
it's pretty.

> +extern const struct i915_power_well_ops i9xx_always_on_power_well_ops;
> +extern const struct i915_power_well_ops chv_pipe_power_well_ops;
> +extern const struct i915_power_well_ops chv_dpio_cmn_power_well_ops;
> +extern const struct i915_power_well_ops i830_pipes_power_well_ops;
> +extern const struct i915_power_well_ops hsw_power_well_ops;
> +extern const struct i915_power_well_ops hsw_power_well_ops;
> +extern const struct i915_power_well_ops

Re: [Intel-gfx] [PATCH v5 3/5] drm/i915: support 64K GTT pages for discrete cards

2022-01-31 Thread Thomas Hellström



On 1/25/22 20:35, Robert Beckett wrote:

From: Matthew Auld 

discrete cards optimise 64K GTT pages for local-memory, since everything
should be allocated at 64K granularity. We say goodbye to sparse
entries, and instead get a compact 256B page-table for 64K pages,
which should be more cache friendly. 4K pages for local-memory
are no longer supported by the HW.

v4: don't return uninitialized err in igt_ppgtt_compact
Reported-by: kernel test robot 


Reviewed-by: Thomas Hellström 




Signed-off-by: Matthew Auld 
Signed-off-by: Stuart Summers 
Signed-off-by: Ramalingam C 
Signed-off-by: Robert Beckett 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
---
  .../gpu/drm/i915/gem/selftests/huge_pages.c   |  60 ++
  drivers/gpu/drm/i915/gt/gen8_ppgtt.c  | 108 +-
  drivers/gpu/drm/i915/gt/intel_gtt.h   |   3 +
  drivers/gpu/drm/i915/gt/intel_ppgtt.c |   1 +
  4 files changed, 169 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c 
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index f36191ebf964..a7d9bdb85d70 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1478,6 +1478,65 @@ static int igt_ppgtt_sanity_check(void *arg)
return err;
  }
  
+static int igt_ppgtt_compact(void *arg)

+{
+   struct drm_i915_private *i915 = arg;
+   struct drm_i915_gem_object *obj;
+   int err;
+
+   /*
+* Simple test to catch issues with compact 64K pages -- since the pt is
+* compacted to 256B that gives us 32 entries per pt, however since the
+* backing page for the pt is 4K, any extra entries we might incorrectly
+* write out should be ignored by the HW. If ever hit such a case this
+* test should catch it since some of our writes would land in scratch.
+*/
+
+   if (!HAS_64K_PAGES(i915)) {
+   pr_info("device lacks compact 64K page support, skipping\n");
+   return 0;
+   }
+
+   if (!HAS_LMEM(i915)) {
+   pr_info("device lacks LMEM support, skipping\n");
+   return 0;
+   }
+
+   /* We want the range to cover multiple page-table boundaries. */
+   obj = i915_gem_object_create_lmem(i915, SZ_4M, 0);
+   if (IS_ERR(obj))
+   return PTR_ERR(obj);
+
+   err = i915_gem_object_pin_pages_unlocked(obj);
+   if (err)
+   goto out_put;
+
+   if (obj->mm.page_sizes.phys < I915_GTT_PAGE_SIZE_64K) {
+   pr_info("LMEM compact unable to allocate huge-page(s)\n");
+   goto out_unpin;
+   }
+
+   /*
+* Disable 2M GTT pages by forcing the page-size to 64K for the GTT
+* insertion.
+*/
+   obj->mm.page_sizes.sg = I915_GTT_PAGE_SIZE_64K;
+
+   err = igt_write_huge(i915, obj);
+   if (err)
+   pr_err("LMEM compact write-huge failed\n");
+
+out_unpin:
+   i915_gem_object_unpin_pages(obj);
+out_put:
+   i915_gem_object_put(obj);
+
+   if (err == -ENOMEM)
+   err = 0;
+
+   return err;
+}
+
  static int igt_tmpfs_fallback(void *arg)
  {
struct drm_i915_private *i915 = arg;
@@ -1735,6 +1794,7 @@ int i915_gem_huge_page_live_selftests(struct 
drm_i915_private *i915)
SUBTEST(igt_tmpfs_fallback),
SUBTEST(igt_ppgtt_smoke_huge),
SUBTEST(igt_ppgtt_sanity_check),
+   SUBTEST(igt_ppgtt_compact),
};
  
  	if (!HAS_PPGTT(i915)) {

diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c 
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index c43e724afa9f..62471730266c 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -233,6 +233,8 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * 
const vm,
   start, end, lvl);
} else {
unsigned int count;
+   unsigned int pte = gen8_pd_index(start, 0);
+   unsigned int num_ptes;
u64 *vaddr;
  
  			count = gen8_pt_count(start, end);

@@ -242,10 +244,18 @@ static u64 __gen8_ppgtt_clear(struct i915_address_space * 
const vm,
atomic_read(&pt->used));
GEM_BUG_ON(!count || count >= atomic_read(&pt->used));
  
+			num_ptes = count;

+   if (pt->is_compact) {
+   GEM_BUG_ON(num_ptes % 16);
+   GEM_BUG_ON(pte % 16);
+   num_ptes /= 16;
+   pte /= 16;
+   }
+
vaddr = px_vaddr(pt);
-   memset64(vaddr + gen8_pd_index(start, 0),
+   memset64(vaddr + pte,
 vm->scratch[0]->encode,
-count);
+  

Re: [Intel-gfx] [PATCH 4/5] drm/i915/dg2: Add Wa_22011100796

2022-01-31 Thread Matthew Auld
On Fri, 28 Jan 2022 at 18:52, Ramalingam C  wrote:
>
> From: Bruce Chang 
>
> Whenever Full soft reset is required, reset all individual engines
> first, and then do a full soft reset.
>
> Signed-off-by: Bruce Chang 
> cc: Matt Roper 
> Cc: Rodrigo Vivi 
> Signed-off-by: Ramalingam C 
Acked-by: Matthew Auld 


Re: [Intel-gfx] [PATCH 2/5] drm/i915: align the plane_vma to min_page_size of stolen mem

2022-01-31 Thread Matthew Auld
On Mon, 31 Jan 2022 at 10:18, Matthew Auld  wrote:
>
> On 28/01/2022 18:52, Ramalingam C wrote:
> > Align the plane vma size to the stolem memory regions' min_page_size.
> >
> > Signed-off-by: Ramalingam C 
> > cc: Matthew Auld 
> > cc: Chris P Wilson 
> Reviewed-by: Matthew Auld 

Do you know for sure that the initial fb is allocated in stolen-lmem on DG2 btw?


Re: [Intel-gfx] [PATCH 3/5] drm/i915: More gt idling time with guc submission

2022-01-31 Thread Matthew Auld

On 28/01/2022 18:52, Ramalingam C wrote:

On i915_selftest@live@gt_timelines, we create many contexts in loop and
create and submit request and then destoy contexts. Destroying the context
needs to disable scheduling, wait for G2H, deregister context and wait
for G2H to destroy each context. Idling of the gt has to wait for all
this to complete which is taking ~3sec for this test.

Hence we are increasing the igt_flush_test's timeout for gt idling to
3Sec.

Signed-off-by: Ramalingam C 
cc: Matthew Brost 

Acked-by: Matthew Auld 


Re: [Intel-gfx] [PATCH 2/5] drm/i915: align the plane_vma to min_page_size of stolen mem

2022-01-31 Thread Matthew Auld

On 28/01/2022 18:52, Ramalingam C wrote:

Align the plane vma size to the stolem memory regions' min_page_size.

Signed-off-by: Ramalingam C 
cc: Matthew Auld 
cc: Chris P Wilson 

Reviewed-by: Matthew Auld 


Re: [Intel-gfx] [PATCH] drm/i915/dg2: s/engine->i915/i915/ for engine workarounds

2022-01-31 Thread Tvrtko Ursulin



On 28/01/2022 17:01, Matt Roper wrote:

rcs_engine_wa_init() has a local 'i915' variable; we should use that
rather than 'engine->i915' for consistency with how we handle other
platforms.

Suggested-by: Tvrtko Ursulin 
Signed-off-by: Matt Roper 


Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko


---
  drivers/gpu/drm/i915/gt/intel_workarounds.c | 30 ++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c 
b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 065dc1c2bb71..3edb1ba6b5cf 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -2045,12 +2045,12 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
struct i915_wa_list *wal)
  {
struct drm_i915_private *i915 = engine->i915;
  
-	if (IS_DG2(engine->i915)) {

+   if (IS_DG2(i915)) {
/* Wa_14015227452:dg2 */
wa_masked_en(wal, GEN9_ROW_CHICKEN4, XEHP_DIS_BBL_SYSPIPE);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0)) {

+   if (IS_DG2_GRAPHICS_STEP(i915, G11, STEP_A0, STEP_B0)) {
/* Wa_14013392000:dg2_g11 */
wa_masked_en(wal, GEN7_ROW_CHICKEN2, 
GEN12_ENABLE_LARGE_GRF_MODE);
  
@@ -2058,15 +2058,15 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)

wa_write_or(wal, LSC_CHICKEN_BIT_0_UDW, DIS_CHAIN_2XSIMD8);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0) ||

-   IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0)) {
+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0) ||
+   IS_DG2_GRAPHICS_STEP(i915, G11, STEP_A0, STEP_B0)) {
/* Wa_14012419201:dg2 */
wa_masked_en(wal, GEN9_ROW_CHICKEN4,
 GEN12_DISABLE_HDR_PAST_PAYLOAD_HOLD_FIX);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_C0) ||

-   IS_DG2_G11(engine->i915)) {
+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) ||
+   IS_DG2_G11(i915)) {
/*
 * Wa_22012826095:dg2
 * Wa_22013059131:dg2
@@ -2081,14 +2081,14 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, 
struct i915_wa_list *wal)
}
  
  	/* Wa_1308578152:dg2_g10 when first gslice is fused off */

-   if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_C0) &&
+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) &&
needs_wa_1308578152(engine)) {
wa_masked_dis(wal, GEN12_CS_DEBUG_MODE1_CCCSUNIT_BE_COMMON,
  GEN12_REPLAY_MODE_GRANULARITY);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_FOREVER) ||

-   IS_DG2_G11(engine->i915)) {
+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_FOREVER) ||
+   IS_DG2_G11(i915)) {
/* Wa_22013037850:dg2 */
wa_write_or(wal, LSC_CHICKEN_BIT_0_UDW,
DISABLE_128B_EVICTION_COMMAND_UDW);
@@ -2105,7 +2105,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct 
i915_wa_list *wal)
  LSC_L1_FLUSH_CTL_3D_DATAPORT_FLUSH_EVENTS_MASK);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0)) {

+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
/*
 * Wa_1608949956:dg2_g10
 * Wa_14010198302:dg2_g10
@@ -2124,7 +2124,7 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct 
i915_wa_list *wal)
   0, false);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_B0)) {

+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) {
/* Wa_22010430635:dg2 */
wa_masked_en(wal,
 GEN9_ROW_CHICKEN4,
@@ -2134,8 +2134,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct 
i915_wa_list *wal)
wa_write_or(wal, XEHP_L3NODEARBCFG, XEHP_LNESPARE);
}
  
-	if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_A0, STEP_C0) ||

-   IS_DG2_G11(engine->i915)) {
+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_C0) ||
+   IS_DG2_G11(i915)) {
/* Wa_22012654132:dg2 */
wa_add(wal, GEN10_CACHE_MODE_SS, 0,
   _MASKED_BIT_ENABLE(ENABLE_PREFETCH_INTO_IC),
@@ -2144,8 +2144,8 @@ rcs_engine_wa_init(struct intel_engine_cs *engine, struct 
i915_wa_list *wal)
}
  
  	/* Wa_14013202645:dg2 */

-   if (IS_DG2_GRAPHICS_STEP(engine->i915, G10, STEP_B0, STEP_C0) ||
-   IS_DG2_GRAPHICS_STEP(engine->i915, G11, STEP_A0, STEP_B0))
+   if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_B0, STEP_C0) ||
+   IS_DG2_GRAPHICS_STEP(i915, G11, STEP_A0, STEP_B0))
wa_write_or(wal, RT_CTRL, DIS_NULL_QUERY);
  
  	if (IS_DG1_GRAPHICS_STEP(i