[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Minor Fixes and Refactoring for HDMI PCON stuff (rev2)
== 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
== 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
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
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
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
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
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
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
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
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
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()
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'?
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
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
== 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
== 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
== 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)
== 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
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
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"
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()
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
== 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)
== 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)
== 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
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
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
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
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
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
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()
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()
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()
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)
== 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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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()
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
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
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
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.
> 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
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
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
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
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
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
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
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
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