[PATCH v2 1/2] drm/display/dsc: Refactor DRM MST DSC Determination Policy
[why] How we determine the dsc_aux used for dsc decompression in drm_dp_mst_dsc_aux_for_port() today has defects: 1. The method how we determine a connected peer device is virtual or not in drm_dp_mst_is_virtual_dpcd() is not always correct. There are DP1.4 products in the market which don't fully comply with DP spec to enumerate virtual peer device. That leads to existing logic defects. For example: - Some 1.4 mst hubs with hdmi output port don't enumerate virtual dpcd/peer device. When probing the hub, its topology is constructed with a branch device only, with peer device type set as DP-to-Legacy converter for its HDMI output port. Under this condition, drm_dp_mst_is_virtual_dpcd() will still determine it's connected with a virtual peer device with virtual dpcd. And results in the section for analyzing DP-to-DP peer device case of drm_dp_mst_dsc_aux_for_port(). That's logically incorrect. 2. Existing routine is designed based on analyzing different connected peer device types, such as dp-dp, dp-hdmi peer device, and virtual sink. Such categorization is redundant and unnecessary. The key info of determining where to do dsc decompression relies on the dsc capability from dpcd only. No matter the mst branch device enumerates virtual dpcd or not, if it's supporting dsc, it must declare it's dsc capability at somewhere within its responded topology. Therefore, we would like to refactor the logic how we determine the dsc aux. [how] 1. dsc_aux should be determined by the topology connection status and dpcd capability info only. In this way, dsc aux could be determined in a more generic way, instead of enumerating and analyzing on different connected peer device types. 2. Synaptics quirk DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD analyzing is no longer needed as long as we determine dsc aux generically by dpcd info. Signed-off-by: Fangzhi Zuo Signed-off-by: Wayne Lin --- drivers/gpu/drm/display/drm_dp_mst_topology.c | 238 -- include/drm/display/drm_dp_mst_helper.h | 3 + 2 files changed, 104 insertions(+), 137 deletions(-) diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index ac90118b9e7a..a4551c17a07f 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -2258,6 +2258,8 @@ void drm_dp_mst_connector_early_unregister(struct drm_connector *connector, drm_dbg_kms(port->mgr->dev, "unregistering %s remote bus for %s\n", port->aux.name, connector->kdev->kobj.name); drm_dp_aux_unregister_devnode(&port->aux); + port->dsc_aux = NULL; + port->passthrough_aux = NULL; } EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister); @@ -5994,57 +5996,6 @@ static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_mst_port *port) i2c_del_adapter(&port->aux.ddc); } -/** - * drm_dp_mst_is_virtual_dpcd() - Is the given port a virtual DP Peer Device - * @port: The port to check - * - * A single physical MST hub object can be represented in the topology - * by multiple branches, with virtual ports between those branches. - * - * As of DP1.4, An MST hub with internal (virtual) ports must expose - * certain DPCD registers over those ports. See sections 2.6.1.1.1 - * and 2.6.1.1.2 of Display Port specification v1.4 for details. - * - * May acquire mgr->lock - * - * Returns: - * true if the port is a virtual DP peer device, false otherwise - */ -static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port) -{ - struct drm_dp_mst_port *downstream_port; - - if (!port || port->dpcd_rev < DP_DPCD_REV_14) - return false; - - /* Virtual DP Sink (Internal Display Panel) */ - if (drm_dp_mst_port_is_logical(port)) - return true; - - /* DP-to-HDMI Protocol Converter */ - if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV && - !port->mcs && - port->ldps) - return true; - - /* DP-to-DP */ - mutex_lock(&port->mgr->lock); - if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING && - port->mstb && - port->mstb->num_ports == 2) { - list_for_each_entry(downstream_port, &port->mstb->ports, next) { - if (downstream_port->pdt == DP_PEER_DEVICE_SST_SINK && - !downstream_port->input) { - mutex_unlock(&port->mgr->lock); - return true; - } - } - } - mutex_unlock(&port->mgr->lock); - - return false; -} - /** * drm_dp_mst_aux_for_parent() - Get the AUX device for an MST port's parent * @port: MST port whose parent's AUX device is returned @@ -6079,115 +6030,128 @@ EXPORT_SYMBOL(drm_dp_mst_aux_for_parent); */ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp
Re: [PATCH 07/17] drm/amd/display: Fix brightness level not retained over reboot
On 11/1/2024 14:48, Mohamed, Zaeem wrote: [AMD Official Use Only - AMD Internal Distribution Only] Hi Mario, Do I need to re-send the patch to amd-gfx after the tags have been added or is sending upstream enough? Zaeem Zaeem, No need to resend to amd-gfx, they can be added when they're committed. If you use "b4" [1] to apply them it will automatically pick them up. If they're applied manually then the person applying them can manually apply the tags too. [1] https://b4.docs.kernel.org/en/latest/ -Original Message- From: Limonciello, Mario Sent: Friday, November 1, 2024 11:43 AM To: Mohamed, Zaeem ; amd-gfx@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Siqueira, Rodrigo ; Mahfooz, Hamza ; Pillai, Aurabindo ; Li, Roman ; Lin, Wayne ; Chung, ChiaHsuan (Tom) ; Zuo, Jerry ; Chiu, Solomon ; Wheeler, Daniel ; mark.herber...@gmail.com Subject: Re: [PATCH 07/17] drm/amd/display: Fix brightness level not retained over reboot On 11/1/2024 08:49, Zaeem Mohamed wrote: From: Tom Chung [Why] During boot up and resume the DC layer will reset the panel brightness to fix a flicker issue. It will cause the dm->actual_brightness is not the current panel brightness level. (the dm->brightness is the correct panel level) [How] Set the backlight level after do the set mode. Reviewed-by: Sun peng Li Signed-off-by: Tom Chung Signed-off-by: Zaeem Mohamed Some more tags, please explicitly add these while merging. Cc: sta...@vger.kernel.org Fixes: d9e865826c20 ("drm/amd/display: Simplify brightness initialization") Reported-by: Mark Herbert Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3655 --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index bbfc47f6595f..2599a99509de 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9411,6 +9411,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, bool mode_set_reset_required = false; u32 i; struct dc_commit_streams_params params = {dc_state->streams, dc_state->stream_count}; + bool set_backlight_level = false; /* Disable writeback */ for_each_old_connector_in_state(state, connector, old_con_state, i) { @@ -9530,6 +9531,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, acrtc->hw_mode = new_crtc_state->mode; crtc->hwmode = new_crtc_state->mode; mode_set_reset_required = true; + set_backlight_level = true; } else if (modereset_required(new_crtc_state)) { drm_dbg_atomic(dev, "Atomic commit: RESET. crtc id %d:[%p]\n", @@ -9581,6 +9583,19 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, acrtc->otg_inst = status->primary_otg_inst; } } + + /* During boot up and resume the DC layer will reset the panel brightness + * to fix a flicker issue. + * It will cause the dm->actual_brightness is not the current panel brightness + * level. (the dm->brightness is the correct panel level) + * So we set the backlight level with dm->brightness value after set mode + */ + if (set_backlight_level) { + for (i = 0; i < dm->num_of_edps; i++) { + if (dm->backlight_dev[i]) + amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]); + } + } } static void dm_set_writeback(struct amdgpu_display_manager *dm,
[PATCH 16/17] drm/amd/display: Prune Invalid Modes For HDMI Output
From: Fangzhi Zuo [Why] 1. HDMI does not have 6 bpc support. Having 6 bpc pass validation does not comply with spec. 2. Validate 420 only for native HDMI, but not apply to pcon use case. 3. Current mode validation log is not readable. [how] 1. Cap 8 bpc for dp-hdmi converter. 2. Validate yuv420 for pcon use case as well, if rgb/yuv444 8bpc cannot fit into pcon bw limitation of the link from the converter to HDMI sink. 3. Add readable pixel_format and color_depth into debug log. Reviewed-by: Wayne Lin Signed-off-by: Fangzhi Zuo Signed-off-by: Zaeem Mohamed --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++--- .../gpu/drm/amd/display/dc/core/dc_debug.c| 40 +++ .../gpu/drm/amd/display/dc/core/dc_stream.c | 6 +-- .../gpu/drm/amd/display/dc/inc/core_status.h | 2 + 4 files changed, 59 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 00e0a10add86..7a1b5d5beeaf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7323,10 +7323,15 @@ create_validate_stream_for_sink(struct amdgpu_dm_connector *aconnector, const struct drm_connector_state *drm_state = dm_state ? &dm_state->base : NULL; int requested_bpc = drm_state ? drm_state->max_requested_bpc : 8; enum dc_status dc_result = DC_OK; + uint8_t bpc_limit = 6; if (!dm_state) return NULL; + if (aconnector->dc_link->connector_signal == SIGNAL_TYPE_HDMI_TYPE_A || + aconnector->dc_link->dpcd_caps.dongle_type == DISPLAY_DONGLE_DP_HDMI_CONVERTER) + bpc_limit = 8; + do { stream = create_stream_for_sink(connector, drm_mode, dm_state, old_stream, @@ -7347,11 +7352,12 @@ create_validate_stream_for_sink(struct amdgpu_dm_connector *aconnector, dc_result = dm_validate_stream_and_context(adev->dm.dc, stream); if (dc_result != DC_OK) { - DRM_DEBUG_KMS("Mode %dx%d (clk %d) failed DC validation with error %d (%s)\n", + DRM_DEBUG_KMS("Mode %dx%d (clk %d) pixel_encoding:%s color_depth:%s failed validation -- %s\n", drm_mode->hdisplay, drm_mode->vdisplay, drm_mode->clock, - dc_result, + dc_pixel_encoding_to_str(stream->timing.pixel_encoding), + dc_color_depth_to_str(stream->timing.display_color_depth), dc_status_to_str(dc_result)); dc_stream_release(stream); @@ -7359,10 +7365,13 @@ create_validate_stream_for_sink(struct amdgpu_dm_connector *aconnector, requested_bpc -= 2; /* lower bpc to retry validation */ } - } while (stream == NULL && requested_bpc >= 6); + } while (stream == NULL && requested_bpc >= bpc_limit); - if (dc_result == DC_FAIL_ENC_VALIDATE && !aconnector->force_yuv420_output) { - DRM_DEBUG_KMS("Retry forcing YCbCr420 encoding\n"); + if ((dc_result == DC_FAIL_ENC_VALIDATE || +dc_result == DC_EXCEED_DONGLE_CAP) && +!aconnector->force_yuv420_output) { + DRM_DEBUG_KMS("%s:%d Retry forcing yuv420 encoding\n", +__func__, __LINE__); aconnector->force_yuv420_output = true; stream = create_validate_stream_for_sink(aconnector, drm_mode, diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c index 0bb25c537243..af1ea5792560 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c @@ -392,3 +392,43 @@ char *dc_status_to_str(enum dc_status status) return "Unexpected status error"; } + +char *dc_pixel_encoding_to_str(enum dc_pixel_encoding pixel_encoding) +{ + switch (pixel_encoding) { + case PIXEL_ENCODING_RGB: + return "RGB"; + case PIXEL_ENCODING_YCBCR422: + return "YUV422"; + case PIXEL_ENCODING_YCBCR444: + return "YUV444"; + case PIXEL_ENCODING_YCBCR420: + return "YUV420"; + default: + return "Unknown"; + } +} + +char *dc_color_depth_to_str(enum dc_color_depth color_depth) +{ + switch (color_depth) { + case COLOR_DEPTH_666: + return "6-bpc"; + case COLOR_DEPTH_888: + return "8-bpc"; + case COLOR_DEPTH_101010: + return "10-bpc"; + case COLOR_DEPTH_121212: + return "12-bpc"; + case COLOR_DEPTH_141414: + retur
[PATCH 08/17] drm/amd/display: SPL cleanup
From: Samson Tam [Why & How] Move from pointer to callback to reference callback directly Missed renaming fixpt functions with spl prefix Reviewed-by: Navid Assadian Signed-off-by: Samson Tam Signed-off-by: Zaeem Mohamed --- .../gpu/drm/amd/display/dc/dc_spl_translate.c | 14 drivers/gpu/drm/amd/display/dc/spl/dc_spl.c | 2 +- .../gpu/drm/amd/display/dc/spl/dc_spl_types.h | 4 +-- .../drm/amd/display/dc/spl/spl_fixpt31_32.c | 34 +-- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc_spl_translate.c b/drivers/gpu/drm/amd/display/dc/dc_spl_translate.c index 24aa9df892f3..c8d8e335fa37 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_spl_translate.c +++ b/drivers/gpu/drm/amd/display/dc/dc_spl_translate.c @@ -8,13 +8,13 @@ #include "dcn32/dcn32_dpp.h" #include "dcn401/dcn401_dpp.h" -static struct spl_funcs dcn2_spl_funcs = { +static struct spl_callbacks dcn2_spl_callbacks = { .spl_calc_lb_num_partitions = dscl2_spl_calc_lb_num_partitions, }; -static struct spl_funcs dcn32_spl_funcs = { +static struct spl_callbacks dcn32_spl_callbacks = { .spl_calc_lb_num_partitions = dscl32_spl_calc_lb_num_partitions, }; -static struct spl_funcs dcn401_spl_funcs = { +static struct spl_callbacks dcn401_spl_callbacks = { .spl_calc_lb_num_partitions = dscl401_spl_calc_lb_num_partitions, }; static void populate_splrect_from_rect(struct spl_rect *spl_rect, const struct rect *rect) @@ -77,16 +77,16 @@ void translate_SPL_in_params_from_pipe_ctx(struct pipe_ctx *pipe_ctx, struct spl // This is used to determine the vtap support switch (plane_state->ctx->dce_version) { case DCN_VERSION_2_0: - spl_in->funcs = &dcn2_spl_funcs; + spl_in->callbacks = dcn2_spl_callbacks; break; case DCN_VERSION_3_2: - spl_in->funcs = &dcn32_spl_funcs; + spl_in->callbacks = dcn32_spl_callbacks; break; case DCN_VERSION_4_01: - spl_in->funcs = &dcn401_spl_funcs; + spl_in->callbacks = dcn401_spl_callbacks; break; default: - spl_in->funcs = &dcn2_spl_funcs; + spl_in->callbacks = dcn2_spl_callbacks; } // Make format field from spl_in point to plane_res scl_data format spl_in->basic_in.format = (enum spl_pixel_format)pipe_ctx->plane_res.scl_data.format; diff --git a/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c b/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c index 9095da7b842b..a29a9f131e04 100644 --- a/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c +++ b/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c @@ -981,7 +981,7 @@ static bool spl_get_optimal_number_of_taps( else lb_config = LB_MEMORY_CONFIG_0; // Determine max vtap support by calculating how much line buffer can fit - spl_in->funcs->spl_calc_lb_num_partitions(spl_in->basic_out.alpha_en, &spl_scratch->scl_data, + spl_in->callbacks.spl_calc_lb_num_partitions(spl_in->basic_out.alpha_en, &spl_scratch->scl_data, lb_config, &num_part_y, &num_part_c); /* MAX_V_TAPS = MIN (NUM_LINES - MAX(CEILING(V_RATIO,1)-2, 0), 8) */ if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.vert) > 2) diff --git a/drivers/gpu/drm/amd/display/dc/spl/dc_spl_types.h b/drivers/gpu/drm/amd/display/dc/spl/dc_spl_types.h index 8b00ccb1dfda..55d557df4aa5 100644 --- a/drivers/gpu/drm/amd/display/dc/spl/dc_spl_types.h +++ b/drivers/gpu/drm/amd/display/dc/spl/dc_spl_types.h @@ -497,7 +497,7 @@ enum scale_to_sharpness_policy { SCALE_TO_SHARPNESS_ADJ_YUV = 1, SCALE_TO_SHARPNESS_ADJ_ALL = 2 }; -struct spl_funcs { +struct spl_callbacks { void (*spl_calc_lb_num_partitions) (bool alpha_en, const struct spl_scaler_data *scl_data, @@ -518,7 +518,7 @@ struct spl_in { // Basic slice information int odm_slice_index;// ODM Slice Index using get_odm_split_index struct spl_taps scaling_quality; // Explicit Scaling Quality - struct spl_funcs *funcs; + struct spl_callbacks callbacks; // Inputs for isharp and EASF struct adaptive_sharpness adaptive_sharpness; // Adaptive Sharpness enum linear_light_scaling lls_pref; // Linear Light Scaling diff --git a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c index 5fd79d9c67e2..131f1e3949d3 100644 --- a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c +++ b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c @@ -22,7 +22,7 @@ static inline unsigned long long abs_i64( * result = dividend / divisor * *remainder = dividend % divisor */ -static inline unsigned long long complete_integer_division_u64( +static inline unsigned long long spl_complete_integer_division_u64( unsigned long
[PATCH 00/17] DC Patches October 28, 2024
This DC patchset brings improvements in multiple areas. In summary, we have: - Prune Invalid Modes for HDMI Output - SPL Cleanup - Fix brightness level not retained over reboot - Remove inaccessible registers from DMU diagnostics Cc: Daniel Wheeler Aric Cyr (1): drm/amd/display: 3.2.308 Aurabindo Pillai (1): drm/amd/display: parse umc_info or vram_info based on ASIC Ausef Yousof (3): Revert "drm/amd/display: Block UHBR Based On USB-C PD Cable ID" drm/amd/display: Remove hw w/a toggle if on DP2/HPO drm/amd/display: Remove otg w/a toggling on HPO interfaces Austin Zheng (1): drm/amd/display: Do Not Fallback To SW Cursor If HW Cursor Required Charlene Liu (1): drm/amd/display: avoid divided by zero Dominik Kaszewski (1): drm/amd/display: fix rxstatus_msg_sz type narrowing Fangzhi Zuo (1): drm/amd/display: Prune Invalid Modes For HDMI Output Ilya Bakoulin (1): drm/amd/display: Minimize wait for pending updates Kaitlyn Tse (1): drm/amd/display: Implement new backlight_level_params structure Nicholas Kazlauskas (1): drm/amd/display: Remove inaccessible registers from DMU diagnostics Samson Tam (2): drm/amd/display: fix asserts in SPL during bootup drm/amd/display: SPL cleanup Taimur Hassan (1): drm/amd/display: [FW Promotion] Release 0.0.241.0 Tom Chung (1): drm/amd/display: Fix brightness level not retained over reboot Wayne Lin (1): drm/amd/display: Don't write DP_MSTM_CTRL after LT .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 57 .../drm/amd/display/dc/bios/bios_parser2.c| 4 +- .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 19 +++- drivers/gpu/drm/amd/display/dc/core/dc.c | 5 +- .../gpu/drm/amd/display/dc/core/dc_debug.c| 40 + .../drm/amd/display/dc/core/dc_link_exports.c | 5 +- .../gpu/drm/amd/display/dc/core/dc_stream.c | 10 ++- drivers/gpu/drm/amd/display/dc/dc.h | 6 +- .../gpu/drm/amd/display/dc/dc_spl_translate.c | 14 +-- drivers/gpu/drm/amd/display/dc/dc_types.h | 27 ++ .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 6 +- .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 16 ++-- .../amd/display/dc/hwss/dcn21/dcn21_hwseq.h | 2 + .../amd/display/dc/hwss/dcn31/dcn31_hwseq.c | 49 +++ .../amd/display/dc/hwss/dcn31/dcn31_hwseq.h | 3 +- .../amd/display/dc/hwss/dcn31/dcn31_init.c| 2 +- .../amd/display/dc/hwss/dcn314/dcn314_init.c | 2 +- .../amd/display/dc/hwss/dcn32/dcn32_init.c| 2 +- .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c | 1 + .../amd/display/dc/hwss/dcn35/dcn35_init.c| 2 +- .../amd/display/dc/hwss/dcn351/dcn351_init.c | 2 +- .../amd/display/dc/hwss/dcn401/dcn401_init.c | 2 +- .../drm/amd/display/dc/hwss/hw_sequencer.h| 5 -- .../gpu/drm/amd/display/dc/inc/core_status.h | 2 + drivers/gpu/drm/amd/display/dc/inc/link.h | 3 +- .../dc/link/protocols/link_dp_capability.c| 22 ++--- .../link/protocols/link_edp_panel_control.c | 17 ++-- .../link/protocols/link_edp_panel_control.h | 3 +- drivers/gpu/drm/amd/display/dc/spl/dc_spl.c | 88 +++ .../gpu/drm/amd/display/dc/spl/dc_spl_types.h | 4 +- .../drm/amd/display/dc/spl/spl_fixpt31_32.c | 34 +++ .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 7 +- .../gpu/drm/amd/display/dmub/src/dmub_dcn35.c | 5 +- .../amd/display/modules/freesync/freesync.c | 3 + .../display/modules/hdcp/hdcp2_execution.c| 31 +++ 35 files changed, 326 insertions(+), 174 deletions(-) -- 2.34.1
[PATCH 04/17] drm/amd/display: fix rxstatus_msg_sz type narrowing
From: Dominik Kaszewski [Why] Code reading rxstatus message size was incorrectly assigning it to uint8_t, despite the value being 10 bits long (lower byte plus lowest 2 bits from upper byte). This caused the highest 2 bits to be ignored, potentially missing invalid values. [How] Change all local variables holding rxstatus message size from uint8_t to uint16_t, as in mod_hdcp_message_hdcp2::rx_id_list_size. Replaced untyped HDCP_2_2_HMID_RXSTATUS_MSG_SZ_HI macro with function hdcp_2_2_hmid_rxstatus_msg_sz(const uint8_t[2]) to encapsulate entire calculation and return a typed result. Removed spaces mixed with tabs to fix indentation on modified lines. Reviewed-by: Wenjing Liu Signed-off-by: Dominik Kaszewski Signed-off-by: Zaeem Mohamed --- .../display/modules/hdcp/hdcp2_execution.c| 31 +++ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c index c996365e84b0..1d41dd58f6bc 100644 --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp2_execution.c @@ -27,6 +27,11 @@ #include "hdcp.h" +static inline uint16_t get_hdmi_rxstatus_msg_size(const uint8_t rxstatus[2]) +{ + return HDCP_2_2_HDMI_RXSTATUS_MSG_SZ_HI(rxstatus[1]) << 8 | rxstatus[0]; +} + static inline enum mod_hdcp_status check_receiver_id_list_ready(struct mod_hdcp *hdcp) { uint8_t is_ready = 0; @@ -35,8 +40,7 @@ static inline enum mod_hdcp_status check_receiver_id_list_ready(struct mod_hdcp is_ready = HDCP_2_2_DP_RXSTATUS_READY(hdcp->auth.msg.hdcp2.rxstatus_dp) ? 1 : 0; else is_ready = (HDCP_2_2_HDMI_RXSTATUS_READY(hdcp->auth.msg.hdcp2.rxstatus[1]) && - (HDCP_2_2_HDMI_RXSTATUS_MSG_SZ_HI(hdcp->auth.msg.hdcp2.rxstatus[1]) << 8 | - hdcp->auth.msg.hdcp2.rxstatus[0])) ? 1 : 0; + get_hdmi_rxstatus_msg_size(hdcp->auth.msg.hdcp2.rxstatus) != 0) ? 1 : 0; return is_ready ? MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_HDCP2_RX_ID_LIST_NOT_READY; } @@ -84,15 +88,13 @@ static inline enum mod_hdcp_status check_link_integrity_failure_dp( static enum mod_hdcp_status check_ake_cert_available(struct mod_hdcp *hdcp) { enum mod_hdcp_status status; - uint16_t size; if (is_dp_hdcp(hdcp)) { status = MOD_HDCP_STATUS_SUCCESS; } else { status = mod_hdcp_read_rxstatus(hdcp); if (status == MOD_HDCP_STATUS_SUCCESS) { - size = HDCP_2_2_HDMI_RXSTATUS_MSG_SZ_HI(hdcp->auth.msg.hdcp2.rxstatus[1]) << 8 | - hdcp->auth.msg.hdcp2.rxstatus[0]; + const uint16_t size = get_hdmi_rxstatus_msg_size(hdcp->auth.msg.hdcp2.rxstatus); status = (size == sizeof(hdcp->auth.msg.hdcp2.ake_cert)) ? MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_HDCP2_AKE_CERT_PENDING; @@ -104,7 +106,6 @@ static enum mod_hdcp_status check_ake_cert_available(struct mod_hdcp *hdcp) static enum mod_hdcp_status check_h_prime_available(struct mod_hdcp *hdcp) { enum mod_hdcp_status status; - uint8_t size; status = mod_hdcp_read_rxstatus(hdcp); if (status != MOD_HDCP_STATUS_SUCCESS) @@ -115,8 +116,7 @@ static enum mod_hdcp_status check_h_prime_available(struct mod_hdcp *hdcp) MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_HDCP2_H_PRIME_PENDING; } else { - size = HDCP_2_2_HDMI_RXSTATUS_MSG_SZ_HI(hdcp->auth.msg.hdcp2.rxstatus[1]) << 8 | - hdcp->auth.msg.hdcp2.rxstatus[0]; + const uint16_t size = get_hdmi_rxstatus_msg_size(hdcp->auth.msg.hdcp2.rxstatus); status = (size == sizeof(hdcp->auth.msg.hdcp2.ake_h_prime)) ? MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_HDCP2_H_PRIME_PENDING; @@ -128,7 +128,6 @@ static enum mod_hdcp_status check_h_prime_available(struct mod_hdcp *hdcp) static enum mod_hdcp_status check_pairing_info_available(struct mod_hdcp *hdcp) { enum mod_hdcp_status status; - uint8_t size; status = mod_hdcp_read_rxstatus(hdcp); if (status != MOD_HDCP_STATUS_SUCCESS) @@ -139,8 +138,7 @@ static enum mod_hdcp_status check_pairing_info_available(struct mod_hdcp *hdcp) MOD_HDCP_STATUS_SUCCESS : MOD_HDCP_STATUS_HDCP2_PAIRING_INFO_PENDING; } else { - size = HDCP_2_2_HDMI_RXSTATUS_MSG_SZ_HI(hdcp->auth.msg.hdcp2.rxstatus[1]) << 8 | - hdcp->auth.msg.hdcp2.rxstatus[0]; + const uin
[PATCH 03/17] Revert "drm/amd/display: Block UHBR Based On USB-C PD Cable ID"
From: Ausef Yousof This reverts commit f767e41a6f6b70cd222edd24549c1fa753c550fc. [why & how] The offending commit caused a lighting issue for Samsung Odyssey G9 monitors when connecting via USB-C. The commit was intended to block certain UHBR rates. Reviewed-by: Charlene Liu Signed-off-by: Ausef Yousof Signed-off-by: Zaeem Mohamed --- .../dc/link/protocols/link_dp_capability.c| 22 +-- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c index 93b81918216d..72ef0c3a7ebd 100644 --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_capability.c @@ -1417,8 +1417,7 @@ static bool get_usbc_cable_id(struct dc_link *link, union dp_cable_id *cable_id) if (!link->ctx->dmub_srv || link->ep_type != DISPLAY_ENDPOINT_PHY || - link->link_enc->features.flags.bits.DP_IS_USB_C == 0 || - link->link_enc->features.flags.bits.IS_DP2_CAPABLE == 0) + link->link_enc->features.flags.bits.DP_IS_USB_C == 0) return false; memset(&cmd, 0, sizeof(cmd)); @@ -1431,9 +1430,7 @@ static bool get_usbc_cable_id(struct dc_link *link, union dp_cable_id *cable_id) cable_id->raw = cmd.cable_id.data.output_raw; DC_LOG_DC("usbc_cable_id = %d.\n", cable_id->raw); } - - ASSERT(cmd.cable_id.header.ret_status); - return true; + return cmd.cable_id.header.ret_status == 1; } static void retrieve_cable_id(struct dc_link *link) @@ -2130,8 +2127,6 @@ struct dc_link_settings dp_get_max_link_cap(struct dc_link *link) /* get max link encoder capability */ if (link_enc) link_enc->funcs->get_max_link_cap(link_enc, &max_link_cap); - else - return max_link_cap; /* Lower link settings based on sink's link cap */ if (link->reported_link_cap.lane_count < max_link_cap.lane_count) @@ -2165,15 +2160,10 @@ struct dc_link_settings dp_get_max_link_cap(struct dc_link *link) */ cable_max_link_rate = get_cable_max_link_rate(link); - if (!link->dc->debug.ignore_cable_id) { - if (cable_max_link_rate != LINK_RATE_UNKNOWN) - // cable max link rate known - max_link_cap.link_rate = MIN(max_link_cap.link_rate, cable_max_link_rate); - else if (link_enc->funcs->is_in_alt_mode && link_enc->funcs->is_in_alt_mode(link_enc)) - // cable max link rate ambiguous, DP alt mode, limit to HBR3 - max_link_cap.link_rate = MIN(max_link_cap.link_rate, LINK_RATE_HIGH3); - //else {} - // cable max link rate ambiguous, DP, do nothing + if (!link->dc->debug.ignore_cable_id && + cable_max_link_rate != LINK_RATE_UNKNOWN) { + if (cable_max_link_rate < max_link_cap.link_rate) + max_link_cap.link_rate = cable_max_link_rate; if (!link->dpcd_caps.cable_id.bits.UHBR13_5_CAPABILITY && link->dpcd_caps.cable_id.bits.CABLE_TYPE >= 2) -- 2.34.1
[PATCH 11/17] drm/amd/display: parse umc_info or vram_info based on ASIC
From: Aurabindo Pillai An upstream bug report suggests that there are production dGPUs that are older than DCN401 but still have a umc_info in VBIOS tables with the same version as expected for a DCN401 product. Hence, reading this tables should be guarded with a version check. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3678 Reviewed-by: Dillon Varone Signed-off-by: Aurabindo Pillai Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c index 0d8498ab9b23..be8fbb04ad98 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c @@ -3127,7 +3127,9 @@ static enum bp_result bios_parser_get_vram_info( struct atom_data_revision revision; // vram info moved to umc_info for DCN4x - if (info && DATA_TABLES(umc_info)) { + if (dcb->ctx->dce_version >= DCN_VERSION_4_01 && + dcb->ctx->dce_version < DCN_VERSION_MAX && + info && DATA_TABLES(umc_info)) { header = GET_IMAGE(struct atom_common_table_header, DATA_TABLES(umc_info)); -- 2.34.1
[PATCH 10/17] drm/amd/display: Remove otg w/a toggling on HPO interfaces
From: Ausef Yousof [why&how] Adjust otg w/a disable condition to include HPO explicitly rather than assuming it is implicitly used through DP2. Reviewed-by: Charlene Liu Signed-off-by: Ausef Yousof Signed-off-by: Zaeem Mohamed --- .../drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c index 07b49b4030f9..b77333817f18 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c @@ -151,7 +151,15 @@ static void dcn35_disable_otg_wa(struct clk_mgr *clk_mgr_base, struct dc_state * new_pipe->stream_res.stream_enc && new_pipe->stream_res.stream_enc->funcs->is_fifo_enabled && new_pipe->stream_res.stream_enc->funcs->is_fifo_enabled(new_pipe->stream_res.stream_enc); - bool has_active_hpo = dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(old_pipe) && dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(new_pipe); + + bool has_active_hpo = false; + + if (old_pipe->stream && new_pipe->stream && old_pipe->stream == new_pipe->stream) { + has_active_hpo = dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(old_pipe) && + dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(new_pipe); + +} + if (!has_active_hpo && !dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(pipe) && (pipe->stream && (pipe->stream->dpms_off || dc_is_virtual_signal(pipe->stream->signal) || -- 2.34.1
[PATCH RESEND v9 1/2] drm/atomic: Let drivers decide which planes to async flip
Currently, DRM atomic uAPI allows only primary planes to be flipped asynchronously. However, each driver might be able to perform async flips in other different plane types. To enable drivers to set their own restrictions on which type of plane they can or cannot flip, use the existing atomic_async_check() from struct drm_plane_helper_funcs to enhance this flexibility, thus allowing different plane types to be able to do async flips as well. In order to prevent regressions and such, we keep the current policy: we skip the driver check for the primary plane, because it is always allowed to do async flips on it. Signed-off-by: André Almeida --- Changes from v8: - Rebased on top of 6.12-rc1 --- drivers/gpu/drm/drm_atomic_uapi.c | 39 +-- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 370dc676e3aa543c9827b50df20df78f02b738c9..a0120df4b63e6b3419b53eb3d3673882559501c6 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -27,8 +27,9 @@ * Daniel Vetter */ -#include #include +#include +#include #include #include #include @@ -1063,6 +1064,7 @@ int drm_atomic_set_property(struct drm_atomic_state *state, struct drm_plane *plane = obj_to_plane(obj); struct drm_plane_state *plane_state; struct drm_mode_config *config = &plane->dev->mode_config; + const struct drm_plane_helper_funcs *plane_funcs = plane->helper_private; plane_state = drm_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) { @@ -1070,15 +1072,32 @@ int drm_atomic_set_property(struct drm_atomic_state *state, break; } - if (async_flip && - (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY || -(prop != config->prop_fb_id && - prop != config->prop_in_fence_fd && - prop != config->prop_fb_damage_clips))) { - ret = drm_atomic_plane_get_property(plane, plane_state, - prop, &old_val); - ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); - break; + if (async_flip) { + /* check if the prop does a nop change */ + if ((plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) || + (prop != config->prop_fb_id && +prop != config->prop_in_fence_fd && +prop != config->prop_fb_damage_clips)) { + ret = drm_atomic_plane_get_property(plane, plane_state, + prop, &old_val); + ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop); + break; + } + + /* ask the driver if this non-primary plane is supported */ + if (plane->type != DRM_PLANE_TYPE_PRIMARY) { + ret = -EINVAL; + + if (plane_funcs && plane_funcs->atomic_async_check) + ret = plane_funcs->atomic_async_check(plane, state); + + if (ret) { + drm_dbg_atomic(prop->dev, + "[PLANE:%d] does not support async flips\n", + obj->id); + break; + } + } } ret = drm_atomic_plane_set_property(plane, -- 2.47.0
[PATCH RESEND v9 2/2] drm/amdgpu: Enable async flip on overlay planes
amdgpu can handle async flips on overlay planes, so allow it for atomic async checks. Signed-off-by: André Almeida --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index 495e3cd70426db0182cb2811bc6d5d09f52f8a4b..4c6aed5ca777d76245f5f2865046f0f598be342a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -1266,8 +1266,7 @@ static int amdgpu_dm_plane_atomic_async_check(struct drm_plane *plane, struct drm_plane_state *new_plane_state; struct dm_crtc_state *dm_new_crtc_state; - /* Only support async updates on cursor planes. */ - if (plane->type != DRM_PLANE_TYPE_CURSOR) + if (plane->type != DRM_PLANE_TYPE_CURSOR && plane->type != DRM_PLANE_TYPE_OVERLAY) return -EINVAL; new_plane_state = drm_atomic_get_new_plane_state(state, plane); -- 2.47.0
[PATCH RESEND v9 0/2] drm/atomic: Ease async flip restrictions
Hi, As per my previous patchsets, the goal of this work is to find a nice way to allow amdgpu to perform async page flips in the overlay plane as well, not only on the primary one. Currently, when using the atomic uAPI, this is the only type of plane allowed to do async flips, and every driver accepts it. In my last version, I had created a static field `bool async_flip` for drm_planes. When creating new planes, drivers could tell if such plane was allowed or not to do async flips. This would be latter checked on the atomic uAPI whenever the DRM_MODE_PAGE_FLIP_ASYNC was present. However, Dmitry Baryshkov raised a valid point about getting confused with the existing atomic_async_check() code, giving that is a function to do basically what I want: to let drivers tell DRM whether a giving plane can do async flips or not. It turns out atomic_async_check() is implemented by drivers to deal with the legacy cursor update, so it's not wired with the atomic uAPI because is something that precedes such API. So my new proposal is to just reuse this same function in the atomic uAPI path. The plane restrictions defined at atomic_async_check() should work in this codepath as well. And I will be able to allow overlays planes by modifying amdgpu_dm_plane_atomic_async_check(), and anyone else have a proper place to play with async plane restrictions as well. One note is that currently we always allow async flips for primary planes, regardless of the drivers, but not every atomic_async_check() implementation allows primary planes (because they were writing targeting cursor planes anyway...). To avoid regressions, my patch only calls atomic_async_check() for non primary planes, and always allows primary ones. Thoughts? Changelog v8: https://lore.kernel.org/lkml/20240806135300.114469-1-andrealm...@igalia.com/ - Rebased on top of 6.12-rc1 (drm/drm-next) Changelog v7: https://lore.kernel.org/dri-devel/20240618030024.500532-1-andrealm...@igalia.com/ - Complete rewrite --- Changes in v10: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v9: https://lore.kernel.org/r/20241002-tonyk-async_flip-v9-0-453b1b897...@igalia.com --- André Almeida (2): drm/atomic: Let drivers decide which planes to async flip drm/amdgpu: Enable async flip on overlay planes .../drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c| 3 +- drivers/gpu/drm/drm_atomic_uapi.c | 39 -- 2 files changed, 30 insertions(+), 12 deletions(-) --- base-commit: 81983758430957d9a5cbfe324fd70cf63e7e change-id: 20241002-tonyk-async_flip-828cfe9cf3ca Best regards, -- André Almeida
Re: [PATCH 07/17] drm/amd/display: Fix brightness level not retained over reboot
On 11/1/2024 08:49, Zaeem Mohamed wrote: From: Tom Chung [Why] During boot up and resume the DC layer will reset the panel brightness to fix a flicker issue. It will cause the dm->actual_brightness is not the current panel brightness level. (the dm->brightness is the correct panel level) [How] Set the backlight level after do the set mode. Reviewed-by: Sun peng Li Signed-off-by: Tom Chung Signed-off-by: Zaeem Mohamed Some more tags, please explicitly add these while merging. Cc: sta...@vger.kernel.org Fixes: d9e865826c20 ("drm/amd/display: Simplify brightness initialization") Reported-by: Mark Herbert Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3655 --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index bbfc47f6595f..2599a99509de 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9411,6 +9411,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, bool mode_set_reset_required = false; u32 i; struct dc_commit_streams_params params = {dc_state->streams, dc_state->stream_count}; + bool set_backlight_level = false; /* Disable writeback */ for_each_old_connector_in_state(state, connector, old_con_state, i) { @@ -9530,6 +9531,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, acrtc->hw_mode = new_crtc_state->mode; crtc->hwmode = new_crtc_state->mode; mode_set_reset_required = true; + set_backlight_level = true; } else if (modereset_required(new_crtc_state)) { drm_dbg_atomic(dev, "Atomic commit: RESET. crtc id %d:[%p]\n", @@ -9581,6 +9583,19 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, acrtc->otg_inst = status->primary_otg_inst; } } + + /* During boot up and resume the DC layer will reset the panel brightness +* to fix a flicker issue. +* It will cause the dm->actual_brightness is not the current panel brightness +* level. (the dm->brightness is the correct panel level) +* So we set the backlight level with dm->brightness value after set mode +*/ + if (set_backlight_level) { + for (i = 0; i < dm->num_of_edps; i++) { + if (dm->backlight_dev[i]) + amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]); + } + } } static void dm_set_writeback(struct amdgpu_display_manager *dm,
[PATCH 09/17] drm/amd/display: Remove hw w/a toggle if on DP2/HPO
From: Ausef Yousof [why&how] Applying a hw w/a only relevant to DIG FIFO causing corruption using HPO, do not apply the w/a if on DP2/HPO Reviewed-by: Charlene Liu Signed-off-by: Ausef Yousof Signed-off-by: Zaeem Mohamed --- .../drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c index 1d4fe0de0f67..07b49b4030f9 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c @@ -133,6 +133,8 @@ static void dcn35_disable_otg_wa(struct clk_mgr *clk_mgr_base, struct dc_state * for (i = 0; i < dc->res_pool->pipe_count; ++i) { struct pipe_ctx *old_pipe = &dc->current_state->res_ctx.pipe_ctx[i]; struct pipe_ctx *new_pipe = &context->res_ctx.pipe_ctx[i]; + struct clk_mgr_internal *clk_mgr_internal = TO_CLK_MGR_INTERNAL(clk_mgr_base); + struct dccg *dccg = clk_mgr_internal->dccg; struct pipe_ctx *pipe = safe_to_lower ? &context->res_ctx.pipe_ctx[i] : &dc->current_state->res_ctx.pipe_ctx[i]; @@ -149,8 +151,13 @@ static void dcn35_disable_otg_wa(struct clk_mgr *clk_mgr_base, struct dc_state * new_pipe->stream_res.stream_enc && new_pipe->stream_res.stream_enc->funcs->is_fifo_enabled && new_pipe->stream_res.stream_enc->funcs->is_fifo_enabled(new_pipe->stream_res.stream_enc); - if (pipe->stream && (pipe->stream->dpms_off || dc_is_virtual_signal(pipe->stream->signal) || - !pipe->stream->link_enc) && !stream_changed_otg_dig_on) { + bool has_active_hpo = dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(old_pipe) && dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(new_pipe); + + if (!has_active_hpo && !dccg->ctx->dc->link_srv->dp_is_128b_132b_signal(pipe) && + (pipe->stream && (pipe->stream->dpms_off || dc_is_virtual_signal(pipe->stream->signal) || + !pipe->stream->link_enc) && !stream_changed_otg_dig_on)) { + + /* This w/a should not trigger when we have a dig active */ if (disable) { if (pipe->stream_res.tg && pipe->stream_res.tg->funcs->immediate_disable_crtc) -- 2.34.1
Re: [PATCH v2] amdgpu: prevent NULL pointer dereference if ATIF is not supported
On 10/31/2024 15:50, Antonio Quartulli wrote: On 31/10/2024 20:37, Mario Limonciello wrote: On 10/31/2024 10:28, Antonio Quartulli wrote: acpi_evaluate_object() may return AE_NOT_FOUND (failure), which would result in dereferencing buffer.pointer (obj) while being NULL. Although this case may be unrealistic for the current code, it is still better to protect against possible bugs. Bail out also when status is AE_NOT_FOUND. This fixes 1 FORWARD_NULL issue reported by Coverity Report: CID 1600951: Null pointer dereferences (FORWARD_NULL) Signed-off-by: Antonio Quartulli Can you please dig up the right Fixes: tag? Fixes: c9b7c809b89f ("drm/amd: Guard against bad data for ATIF ACPI method") Your commit :) Should I send v3 with the Fixes tag in it? Don't worry about it, I'll pick it up while we commit it. Thanks! Interestingly, this pattern of checking for AE_NOT_FOUND is shared by other functions, however, they don't try to dereference the pointer to the buffer before the return statement (which caused the Coverity report). It's the caller that checks if the return value is NULL or not. For this function it was the same, until you added this extra check on obj->type, without checking if obj was NULL or not. If we want to keep the original pattern and continue checking for AE_NOT_FOUND, we could rather do: - if (obj->type != ACPI_TYPE_BUFFER) { + if (obj && obj->type != ACPI_TYPE_BUFFER) { But this feel more like "bike shed color picking" than anything else :) Anyway, up to you Mario, I am open to change the patch again if the latter pattern is more preferable. Regards, Besides that, LGTM. Reviewed-by: Mario Limonciello --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/ drm/amd/amdgpu/amdgpu_acpi.c index cce85389427f..b8d4e07d2043 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -172,8 +172,8 @@ static union acpi_object *amdgpu_atif_call(struct amdgpu_atif *atif, &buffer); obj = (union acpi_object *)buffer.pointer; - /* Fail if calling the method fails and ATIF is supported */ - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { + /* Fail if calling the method fails */ + if (ACPI_FAILURE(status)) { DRM_DEBUG_DRIVER("failed to evaluate ATIF got %s\n", acpi_format_exception(status)); kfree(obj);
[PATCH] drm/amd/display: Add a missing DCN401 reg definition
Add a mising reg field to the autogenerated header for future use Signed-off-by: Aurabindo Pillai --- drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h index f42a276499cd..5d9d5fea6e06 100644 --- a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h +++ b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h @@ -6199,10 +6199,12 @@ #define DCHUBBUB_CTRL_STATUS__ROB_UNDERFLOW_STATUS__SHIFT 0x1 #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_STATUS__SHIFT 0x2 #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_CLEAR__SHIFT 0x3 +#define DCHUBBUB_CTRL_STATUS__DCHUBBUB_HW_DEBUG__SHIFT 0x4 #define DCHUBBUB_CTRL_STATUS__CSTATE_SWATH_CHK_GOOD_MODE__SHIFT 0x1f #define DCHUBBUB_CTRL_STATUS__ROB_UNDERFLOW_STATUS_MASK 0x0002L #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_STATUS_MASK 0x0004L #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_CLEAR_MASK 0x0008L +#define DCHUBBUB_CTRL_STATUS__DCHUBBUB_HW_DEBUG_MASK 0x3FF0L #define DCHUBBUB_CTRL_STATUS__CSTATE_SWATH_CHK_GOOD_MODE_MASK 0x8000L //DCHUBBUB_TIMEOUT_DETECTION_CTRL1 #define DCHUBBUB_TIMEOUT_DETECTION_CTRL1__DCHUBBUB_TIMEOUT_ERROR_STATUS__SHIFT 0x0 -- 2.39.5
RE: [PATCH] drm/amd/display: Add a missing DCN401 reg definition
[Public] Reviewed-by: Dillon Varone -Original Message- From: Aurabindo Pillai Sent: Friday, November 1, 2024 12:24 PM To: amd-gfx@lists.freedesktop.org Cc: Wentland, Harry ; Siqueira, Rodrigo ; Li, Sun peng (Leo) ; Varone, Dillon ; Pillai, Aurabindo Subject: [PATCH] drm/amd/display: Add a missing DCN401 reg definition Add a mising reg field to the autogenerated header for future use Signed-off-by: Aurabindo Pillai --- drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h index f42a276499cd..5d9d5fea6e06 100644 --- a/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h +++ b/drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_4_1_0_sh_mask.h @@ -6199,10 +6199,12 @@ #define DCHUBBUB_CTRL_STATUS__ROB_UNDERFLOW_STATUS__SHIFT 0x1 #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_STATUS__SHIFT 0x2 #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_CLEAR__SHIFT 0x3 +#define DCHUBBUB_CTRL_STATUS__DCHUBBUB_HW_DEBUG__SHIFT 0x4 #define DCHUBBUB_CTRL_STATUS__CSTATE_SWATH_CHK_GOOD_MODE__SHIFT 0x1f #define DCHUBBUB_CTRL_STATUS__ROB_UNDERFLOW_STATUS_MASK 0x0002L #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_STATUS_MASK 0x0004L #define DCHUBBUB_CTRL_STATUS__ROB_OVERFLOW_CLEAR_MASK 0x0008L +#define DCHUBBUB_CTRL_STATUS__DCHUBBUB_HW_DEBUG_MASK 0x3FF0L #define DCHUBBUB_CTRL_STATUS__CSTATE_SWATH_CHK_GOOD_MODE_MASK 0x8000L //DCHUBBUB_TIMEOUT_DETECTION_CTRL1 #define DCHUBBUB_TIMEOUT_DETECTION_CTRL1__DCHUBBUB_TIMEOUT_ERROR_STATUS__SHIFT 0x0 -- 2.39.5
[PATCH 05/17] drm/amd/display: Remove inaccessible registers from DMU diagnostics
From: Nicholas Kazlauskas [Why] SEC_CNTL isn't readable by x86 and can block Z8 entry if read. [How] Remove the read. Reviewed-by: Ovidiu Bunea Reviewed-by: Charlene Liu Signed-off-by: Nicholas Kazlauskas Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dmub/src/dmub_dcn35.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn35.c b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn35.c index 3be315f1a443..e5e77bd3c31e 100644 --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn35.c +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn35.c @@ -464,7 +464,7 @@ uint32_t dmub_dcn35_get_current_time(struct dmub_srv *dmub) void dmub_dcn35_get_diagnostic_data(struct dmub_srv *dmub, struct dmub_diagnostic_data *diag_data) { - uint32_t is_dmub_enabled, is_soft_reset, is_sec_reset; + uint32_t is_dmub_enabled, is_soft_reset; uint32_t is_traceport_enabled, is_cw6_enabled; if (!dmub || !diag_data) @@ -514,9 +514,6 @@ void dmub_dcn35_get_diagnostic_data(struct dmub_srv *dmub, struct dmub_diagnosti REG_GET(DMCUB_CNTL2, DMCUB_SOFT_RESET, &is_soft_reset); diag_data->is_dmcub_soft_reset = is_soft_reset; - REG_GET(DMCUB_SEC_CNTL, DMCUB_SEC_RESET_STATUS, &is_sec_reset); - diag_data->is_dmcub_secure_reset = is_sec_reset; - REG_GET(DMCUB_CNTL, DMCUB_TRACEPORT_EN, &is_traceport_enabled); diag_data->is_traceport_en = is_traceport_enabled; -- 2.34.1
[PATCH 01/17] drm/amd/display: Do Not Fallback To SW Cursor If HW Cursor Required
From: Austin Zheng [Why/How] Tearing can occur if there is a flip immediate plane and SW cursor. check_subvp_sw_cursor_fallback_req falls back to SW cursor if the stream has the potential to use subVP. Check for fallback not needed if HW cursor is required. e.g. Fullscreen gaming Reviewed-by: Dillon Varone Signed-off-by: Austin Zheng Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c index b000bf39762f..aca2821d546b 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c @@ -290,7 +290,9 @@ bool dc_stream_set_cursor_attributes( * 2. If not subvp high refresh, for single display cases, if resolution is >= 5K and refresh rate < 120hz * 3. If not subvp high refresh, for multi display cases, if resolution is >= 4K and refresh rate < 120hz */ - if (dc->debug.allow_sw_cursor_fallback && attributes->height * attributes->width * 4 > 16384) { + if (dc->debug.allow_sw_cursor_fallback && + attributes->height * attributes->width * 4 > 16384 && + !stream->hw_cursor_req) { if (check_subvp_sw_cursor_fallback_req(dc, stream)) return false; } -- 2.34.1
[PATCH 06/17] drm/amd/display: fix asserts in SPL during bootup
From: Samson Tam [Why] During mode validation, there maybe modes that fail max_downscale_src_width check and scaling_quality taps are 0. This will cause an assert to trigger in spl_set_filters_data() because taps are 0. [How] Move taps calculation for non-adaptive scaling mode to separate function and call it if max_downscale_src_width fails. This will populate taps if scaling_quality taps are 0. Reviewed-by: Alvin Lee Signed-off-by: Samson Tam Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dc/spl/dc_spl.c | 86 - 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c b/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c index 133906e73a65..9095da7b842b 100644 --- a/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c +++ b/drivers/gpu/drm/amd/display/dc/spl/dc_spl.c @@ -868,6 +868,50 @@ static bool spl_get_isharp_en(struct spl_in *spl_in, return enable_isharp; } +/* Calculate number of tap with adaptive scaling off */ +static void spl_get_taps_non_adaptive_scaler( + struct spl_scratch *spl_scratch, const struct spl_taps *in_taps) +{ + if (in_taps->h_taps == 0) { + if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.horz) > 1) + spl_scratch->scl_data.taps.h_taps = spl_min(2 * spl_fixpt_ceil( + spl_scratch->scl_data.ratios.horz), 8); + else + spl_scratch->scl_data.taps.h_taps = 4; + } else + spl_scratch->scl_data.taps.h_taps = in_taps->h_taps; + + if (in_taps->v_taps == 0) { + if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.vert) > 1) + spl_scratch->scl_data.taps.v_taps = spl_min(spl_fixpt_ceil(spl_fixpt_mul_int( + spl_scratch->scl_data.ratios.vert, 2)), 8); + else + spl_scratch->scl_data.taps.v_taps = 4; + } else + spl_scratch->scl_data.taps.v_taps = in_taps->v_taps; + + if (in_taps->v_taps_c == 0) { + if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.vert_c) > 1) + spl_scratch->scl_data.taps.v_taps_c = spl_min(spl_fixpt_ceil(spl_fixpt_mul_int( + spl_scratch->scl_data.ratios.vert_c, 2)), 8); + else + spl_scratch->scl_data.taps.v_taps_c = 4; + } else + spl_scratch->scl_data.taps.v_taps_c = in_taps->v_taps_c; + + if (in_taps->h_taps_c == 0) { + if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.horz_c) > 1) + spl_scratch->scl_data.taps.h_taps_c = spl_min(2 * spl_fixpt_ceil( + spl_scratch->scl_data.ratios.horz_c), 8); + else + spl_scratch->scl_data.taps.h_taps_c = 4; + } else if ((in_taps->h_taps_c % 2) != 0 && in_taps->h_taps_c != 1) + /* Only 1 and even h_taps_c are supported by hw */ + spl_scratch->scl_data.taps.h_taps_c = in_taps->h_taps_c - 1; + else + spl_scratch->scl_data.taps.h_taps_c = in_taps->h_taps_c; +} + /* Calculate optimal number of taps */ static bool spl_get_optimal_number_of_taps( int max_downscale_src_width, struct spl_in *spl_in, struct spl_scratch *spl_scratch, @@ -883,7 +927,7 @@ static bool spl_get_optimal_number_of_taps( if (spl_scratch->scl_data.viewport.width > spl_scratch->scl_data.h_active && max_downscale_src_width != 0 && spl_scratch->scl_data.viewport.width > max_downscale_src_width) { - memcpy(&spl_scratch->scl_data.taps, in_taps, sizeof(struct spl_taps)); + spl_get_taps_non_adaptive_scaler(spl_scratch, in_taps); *enable_easf_v = false; *enable_easf_h = false; *enable_isharp = false; @@ -910,43 +954,9 @@ static bool spl_get_optimal_number_of_taps( * From programming guide: taps = min{ ceil(2*H_RATIO,1), 8} for downscaling * taps = 4 for upscaling */ - if (skip_easf) { - if (in_taps->h_taps == 0) { - if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.horz) > 1) - spl_scratch->scl_data.taps.h_taps = spl_min(2 * spl_fixpt_ceil( - spl_scratch->scl_data.ratios.horz), 8); - else - spl_scratch->scl_data.taps.h_taps = 4; - } else - spl_scratch->scl_data.taps.h_taps = in_taps->h_taps; - if (in_taps->v_taps == 0) { - if (spl_fixpt_ceil(spl_scratch->scl_data.ratios.vert) > 1) - spl_scratch->scl_data.taps.v_taps = spl_min(spl_fixpt_ceil(spl_fixpt_mul_int( - spl_scratch->scl_data.ratios.vert, 2)), 8)
[PATCH 17/17] drm/amd/display: 3.2.308
From: Aric Cyr This version brings along following fixes: - Prune Invalid Modes for HDMI Output - SPL Cleanup - Fix brightness level not retained over reboot - Remove inaccessible registers from DMU diagnostics Signed-off-by: Aric Cyr Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 5cb58ca77890..ad9ce3d0bfcf 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -55,7 +55,7 @@ struct aux_payload; struct set_config_cmd_payload; struct dmub_notification; -#define DC_VER "3.2.307" +#define DC_VER "3.2.308" #define MAX_SURFACES 3 #define MAX_PLANES 6 -- 2.34.1
[PATCH 13/17] drm/amd/display: Don't write DP_MSTM_CTRL after LT
From: Wayne Lin [Why] Observe after suspend/resme, we can't light up mst monitors under specific mst hub. The reason is that driver still writes DPCD DP_MSTM_CTRL after LT. It's forbidden even we write the same value for that dpcd register. [How] We already resume the mst branch device dpcd settings during resume_mst_branch_status(). Leverage drm_dp_mst_topology_queue_probe() to only probe the topology, not calling drm_dp_mst_topology_mgr_resume() which will set DP_MSTM_CTRL as well. Reviewed-by: Jerry Zuo Signed-off-by: Wayne Lin Signed-off-by: Zaeem Mohamed --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 16 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 2599a99509de..f3e8975cb2fa 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3170,8 +3170,7 @@ static int dm_resume(struct amdgpu_ip_block *ip_block) struct dm_atomic_state *dm_state = to_dm_atomic_state(dm->atomic_obj.state); enum dc_connection_type new_connection_type = dc_connection_none; struct dc_state *dc_state; - int i, r, j, ret; - bool need_hotplug = false; + int i, r, j; struct dc_commit_streams_params commit_params = {}; if (dm->dc->caps.ips_support) { @@ -3360,23 +3359,16 @@ static int dm_resume(struct amdgpu_ip_block *ip_block) aconnector->mst_root) continue; - ret = drm_dp_mst_topology_mgr_resume(&aconnector->mst_mgr, true); - - if (ret < 0) { - dm_helpers_dp_mst_stop_top_mgr(aconnector->dc_link->ctx, - aconnector->dc_link); - need_hotplug = true; - } + drm_dp_mst_topology_queue_probe(&aconnector->mst_mgr); } drm_connector_list_iter_end(&iter); - if (need_hotplug) - drm_kms_helper_hotplug_event(ddev); - amdgpu_dm_irq_resume_late(adev); amdgpu_dm_smu_write_watermarks_table(adev); + drm_kms_helper_hotplug_event(ddev); + return 0; } -- 2.34.1
[PATCH 12/17] drm/amd/display: Minimize wait for pending updates
From: Ilya Bakoulin [Why/How] Move the wait for pending updates past prepare_bandwidth if the previous update was not a full update to reduce the average time it takes to complete a full update. Reviewed-by: Dillon Varone Reviewed-by: Alvin Lee Signed-off-by: Ilya Bakoulin Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dc/core/dc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 8a52fef46785..7872c6cabb14 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -3835,7 +3835,7 @@ static void commit_planes_for_stream(struct dc *dc, dc_exit_ips_for_hw_access(dc); dc_z10_restore(dc); - if (update_type == UPDATE_TYPE_FULL) + if (update_type == UPDATE_TYPE_FULL && dc->optimized_required) hwss_process_outstanding_hw_updates(dc, dc->current_state); for (i = 0; i < dc->res_pool->pipe_count; i++) { @@ -3862,6 +3862,9 @@ static void commit_planes_for_stream(struct dc *dc, context_clock_trace(dc, context); } + if (update_type == UPDATE_TYPE_FULL) + hwss_wait_for_outstanding_hw_updates(dc, dc->current_state); + top_pipe_to_program = resource_get_otg_master_for_stream( &context->res_ctx, stream); -- 2.34.1
[PATCH 07/17] drm/amd/display: Fix brightness level not retained over reboot
From: Tom Chung [Why] During boot up and resume the DC layer will reset the panel brightness to fix a flicker issue. It will cause the dm->actual_brightness is not the current panel brightness level. (the dm->brightness is the correct panel level) [How] Set the backlight level after do the set mode. Reviewed-by: Sun peng Li Signed-off-by: Tom Chung Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index bbfc47f6595f..2599a99509de 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9411,6 +9411,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, bool mode_set_reset_required = false; u32 i; struct dc_commit_streams_params params = {dc_state->streams, dc_state->stream_count}; + bool set_backlight_level = false; /* Disable writeback */ for_each_old_connector_in_state(state, connector, old_con_state, i) { @@ -9530,6 +9531,7 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, acrtc->hw_mode = new_crtc_state->mode; crtc->hwmode = new_crtc_state->mode; mode_set_reset_required = true; + set_backlight_level = true; } else if (modereset_required(new_crtc_state)) { drm_dbg_atomic(dev, "Atomic commit: RESET. crtc id %d:[%p]\n", @@ -9581,6 +9583,19 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state *state, acrtc->otg_inst = status->primary_otg_inst; } } + + /* During boot up and resume the DC layer will reset the panel brightness +* to fix a flicker issue. +* It will cause the dm->actual_brightness is not the current panel brightness +* level. (the dm->brightness is the correct panel level) +* So we set the backlight level with dm->brightness value after set mode +*/ + if (set_backlight_level) { + for (i = 0; i < dm->num_of_edps; i++) { + if (dm->backlight_dev[i]) + amdgpu_dm_backlight_set_level(dm, i, dm->brightness[i]); + } + } } static void dm_set_writeback(struct amdgpu_display_manager *dm, -- 2.34.1
[PATCH 02/17] drm/amd/display: avoid divided by zero
From: Charlene Liu [why] insert divided by zero protection Reviewed-by: Alvin Lee Signed-off-by: Charlene Liu Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c index fc4268729017..f980a84dceef 100644 --- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c +++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c @@ -129,6 +129,9 @@ unsigned int mod_freesync_calc_v_total_from_refresh( unsigned int v_total; unsigned int frame_duration_in_ns; + if (refresh_in_uhz == 0) + return stream->timing.v_total; + frame_duration_in_ns = ((unsigned int)(div64_u64((10ULL * 100), refresh_in_uhz))); -- 2.34.1
[PATCH 14/17] drm/amd/display: [FW Promotion] Release 0.0.241.0
From: Taimur Hassan - Add DPCS health check - Update USB4 PHY SSC - Fix FAMS2 SubVP Close to VBlank changes - Create VESA Aux-based backlight control path - Fix PSR1 CRC error during CTS test Signed-off-by: Taimur Hassan Signed-off-by: Zaeem Mohamed --- drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h index 3aa6c60588b5..a9b90fa00b88 100644 --- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h +++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h @@ -495,6 +495,7 @@ struct dmub_feature_caps { uint8_t gecc_enable; uint8_t replay_supported; uint8_t replay_reserved[3]; + uint8_t abm_aux_backlight_support; }; struct dmub_visual_confirm_color { @@ -754,7 +755,7 @@ union dmub_shared_state_ips_driver_signals { uint32_t allow_ips1 : 1; /**< 1 is IPS1 is allowed */ uint32_t allow_ips2 : 1; /**< 1 is IPS1 is allowed */ uint32_t allow_z10 : 1; /**< 1 if Z10 is allowed */ - uint32_t allow_idle : 1; /**< 1 if driver is allowing idle */ + uint32_t allow_idle: 1; /**< 1 if driver is allowing idle */ uint32_t reserved_bits : 27; /**< Reversed bits */ } bits; uint32_t all; @@ -4460,9 +4461,9 @@ struct dmub_cmd_abm_set_backlight_data { uint8_t backlight_control_type; /** -* Explicit padding to 4 byte boundary. +* AUX HW instance. */ - uint8_t pad[1]; + uint8_t aux_inst; /** * Minimum luminance in nits. -- 2.34.1
[PATCH 15/17] drm/amd/display: Implement new backlight_level_params structure
From: Kaitlyn Tse [Why] Implement the new backlight_level_params structure as part of the VBAC framework, the information in this structure is needed to be passed down to the DMCUB to identify the backlight control type, to adjust the backlight of the panel and to perform any required conversions from PWM to nits or vice versa. [How] Modified existing functions to include the new backlight_level_params structure. Reviewed-by: Nicholas Kazlauskas Signed-off-by: Kaitlyn Tse Signed-off-by: Zaeem Mohamed --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++- .../drm/amd/display/dc/core/dc_link_exports.c | 5 +- drivers/gpu/drm/amd/display/dc/dc.h | 4 +- drivers/gpu/drm/amd/display/dc/dc_types.h | 27 ++ .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 6 +-- .../amd/display/dc/hwss/dcn21/dcn21_hwseq.c | 16 +++--- .../amd/display/dc/hwss/dcn21/dcn21_hwseq.h | 2 + .../amd/display/dc/hwss/dcn31/dcn31_hwseq.c | 49 +++ .../amd/display/dc/hwss/dcn31/dcn31_hwseq.h | 3 +- .../amd/display/dc/hwss/dcn31/dcn31_init.c| 2 +- .../amd/display/dc/hwss/dcn314/dcn314_init.c | 2 +- .../amd/display/dc/hwss/dcn32/dcn32_init.c| 2 +- .../amd/display/dc/hwss/dcn35/dcn35_hwseq.c | 1 + .../amd/display/dc/hwss/dcn35/dcn35_init.c| 2 +- .../amd/display/dc/hwss/dcn351/dcn351_init.c | 2 +- .../amd/display/dc/hwss/dcn401/dcn401_init.c | 2 +- .../drm/amd/display/dc/hwss/hw_sequencer.h| 5 -- drivers/gpu/drm/amd/display/dc/inc/link.h | 3 +- .../link/protocols/link_edp_panel_control.c | 17 --- .../link/protocols/link_edp_panel_control.h | 3 +- 20 files changed, 119 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index f3e8975cb2fa..00e0a10add86 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4640,7 +4640,12 @@ static void amdgpu_dm_backlight_set_level(struct amdgpu_display_manager *dm, if (!rc) DRM_DEBUG("DM: Failed to update backlight via AUX on eDP[%d]\n", bl_idx); } else { - rc = dc_link_set_backlight_level(link, brightness, 0); + struct set_backlight_level_params backlight_level_params = { 0 }; + + backlight_level_params.backlight_pwm_u16_16 = brightness; + backlight_level_params.transition_time_in_ms = 0; + + rc = dc_link_set_backlight_level(link, &backlight_level_params); if (!rc) DRM_DEBUG("DM: Failed to update backlight on eDP[%d]\n", bl_idx); } diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c index dfdfe22d9e85..457d60eeb486 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_exports.c @@ -430,11 +430,10 @@ bool dc_link_get_backlight_level_nits(struct dc_link *link, } bool dc_link_set_backlight_level(const struct dc_link *link, - uint32_t backlight_pwm_u16_16, - uint32_t frame_ramp) + struct set_backlight_level_params *backlight_level_params) { return link->dc->link_srv->edp_set_backlight_level(link, - backlight_pwm_u16_16, frame_ramp); + backlight_level_params); } bool dc_link_set_backlight_level_nits(struct dc_link *link, diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index 76130bbf2fe4..5cb58ca77890 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -225,6 +225,7 @@ struct dc_dmub_caps { bool subvp_psr; bool gecc_enable; uint8_t fams_ver; + bool aux_backlight_support; }; struct dc_scl_caps { @@ -2210,8 +2211,7 @@ void dc_link_edp_panel_backlight_power_on(struct dc_link *link, * and 16 bit fractional, where 1.0 is max backlight value. */ bool dc_link_set_backlight_level(const struct dc_link *dc_link, - uint32_t backlight_pwm_u16_16, - uint32_t frame_ramp); + struct set_backlight_level_params *backlight_level_params); /* Set/get nits-based backlight level of an embedded panel (eDP, LVDS). */ bool dc_link_set_backlight_level_nits(struct dc_link *link, diff --git a/drivers/gpu/drm/amd/display/dc/dc_types.h b/drivers/gpu/drm/amd/display/dc/dc_types.h index f0776484abb7..1fd030e3f4be 100644 --- a/drivers/gpu/drm/amd/display/dc/dc_types.h +++ b/drivers/gpu/drm/amd/display/dc/dc_types.h @@ -1303,4 +1303,31 @@ struct dc_commit_streams_params { enum dc_power_source_type power_source; }; +struct set_backlight_level_params { + /* backlight in pwm */ + uint32_t backlight_pwm_u16_16; + /* brightness ramping */ + uint32_t frame_ramp; + /* back
Re: [PATCH v5] drm/amd/pm: correct the workload setting
On Wed, Oct 30, 2024 at 9:17 PM Kenneth Feng wrote: > > Correct the workload setting in order not to mix the setting > with the end user. Update the workload mask accordingly. > > v2: changes as below: > 1. the end user can not erase the workload from driver except default > workload. > 2. always shows the real highest priority workoad to the end user. > 3. the real workload mask is combined with driver workload mask and end user > workload mask. > > v3: apply this to the other ASICs as well. > v4: simplify the code > v5: refine the code based on the review comments. > > Signed-off-by: Kenneth Feng Acked-by: Alex Deucher > --- > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 49 +-- > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 4 +- > .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 5 +- > .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 5 +- > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 5 +- > .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 4 +- > .../gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c | 4 +- > .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 20 ++-- > .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 5 +- > .../drm/amd/pm/swsmu/smu14/smu_v14_0_2_ppt.c | 9 ++-- > drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c| 8 +++ > drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h| 2 + > 12 files changed, 84 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > index 8d4aee4e2287..2e6c2f8dd065 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c > @@ -1261,26 +1261,33 @@ static int smu_sw_init(struct amdgpu_ip_block > *ip_block) > smu->watermarks_bitmap = 0; > smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT; > smu->default_power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT; > + smu->user_dpm_profile.user_workload_mask = 0; > > atomic_set(&smu->smu_power.power_gate.vcn_gated, 1); > atomic_set(&smu->smu_power.power_gate.jpeg_gated, 1); > atomic_set(&smu->smu_power.power_gate.vpe_gated, 1); > atomic_set(&smu->smu_power.power_gate.umsch_mm_gated, 1); > > - smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0; > - smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D] = 1; > - smu->workload_prority[PP_SMC_POWER_PROFILE_POWERSAVING] = 2; > - smu->workload_prority[PP_SMC_POWER_PROFILE_VIDEO] = 3; > - smu->workload_prority[PP_SMC_POWER_PROFILE_VR] = 4; > - smu->workload_prority[PP_SMC_POWER_PROFILE_COMPUTE] = 5; > - smu->workload_prority[PP_SMC_POWER_PROFILE_CUSTOM] = 6; > + smu->workload_priority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0; > + smu->workload_priority[PP_SMC_POWER_PROFILE_FULLSCREEN3D] = 1; > + smu->workload_priority[PP_SMC_POWER_PROFILE_POWERSAVING] = 2; > + smu->workload_priority[PP_SMC_POWER_PROFILE_VIDEO] = 3; > + smu->workload_priority[PP_SMC_POWER_PROFILE_VR] = 4; > + smu->workload_priority[PP_SMC_POWER_PROFILE_COMPUTE] = 5; > + smu->workload_priority[PP_SMC_POWER_PROFILE_CUSTOM] = 6; > > if (smu->is_apu || > - !smu_is_workload_profile_available(smu, > PP_SMC_POWER_PROFILE_FULLSCREEN3D)) > - smu->workload_mask = 1 << > smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT]; > - else > - smu->workload_mask = 1 << > smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D]; > + !smu_is_workload_profile_available(smu, > PP_SMC_POWER_PROFILE_FULLSCREEN3D)) { > + smu->driver_workload_mask = > + 1 << > smu->workload_priority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT]; > + } else { > + smu->driver_workload_mask = > + 1 << > smu->workload_priority[PP_SMC_POWER_PROFILE_FULLSCREEN3D]; > + smu->default_power_profile_mode = > PP_SMC_POWER_PROFILE_FULLSCREEN3D; > + } > > + smu->workload_mask = smu->driver_workload_mask | > + > smu->user_dpm_profile.user_workload_mask; > smu->workload_setting[0] = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT; > smu->workload_setting[1] = PP_SMC_POWER_PROFILE_FULLSCREEN3D; > smu->workload_setting[2] = PP_SMC_POWER_PROFILE_POWERSAVING; > @@ -2354,17 +2361,20 @@ static int smu_switch_power_profile(void *handle, > return -EINVAL; > > if (!en) { > - smu->workload_mask &= ~(1 << smu->workload_prority[type]); > + smu->driver_workload_mask &= ~(1 << > smu->workload_priority[type]); > index = fls(smu->workload_mask); > index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 > : 0; > workload[0] = smu->workload_setting[index]; > } else { > - smu->workload_mask |= (1
RE: [PATCH 00/17] DC Patches October 28, 2024
[AMD Official Use Only - AMD Internal Distribution Only] Hi all, This week this patchset was tested on 4 systems, two dGPU and two APU based, and tested across multiple display and connection types. APU * Single Display eDP -> 1080p 60hz, 2560x1600 120hz, 1920x1200 165hz * Single Display DP -> 4k144hz, 4k240hz * Multi display -> eDP + DP/HDMI/USB-C -> 1080p 60hz eDP + 4k 144hz, 4k 240hz (Includes USB-C to DP/HDMI adapters) * Thunderbolt -> LG Ultrafine 5k * DSC -> Cable Matters 101075 (DP to 3x DP) with 3x 4k60hz displays, HP Hook G2 with 2x 4k60hz displays * USB 4 -> HP Hook G4, Lenovo Thunderbolt Dock, both with 2x 4k60hz DP and 1x 4k60hz HDMI displays * PCON -> Club3D CAC-1085 + 1x 4k 144hz DGPU * Single Display DP -> 4k144hz, 4k240hz * Multiple Display DP -> 4k240hz + 4k144hz * MST (Startech MST14DP123DP [DP to 3x DP] and 2x 4k 60hz displays) * DSC (with Cable Matters 101075 [DP to 3x DP] with 3x 4k60hz displays) The testing is a mix of automated and manual tests. Manual testing includes (but is not limited to) * Changing display configurations and settings * Video/Audio playback * Benchmark testing * Suspend/Resume testing * Feature testing (Freesync, HDCP, etc.) Automated testing includes (but is not limited to) * Script testing (scripts to automate some of the manual checks) * IGT testing The testing is mainly tested on the following displays, but occasionally there are tests with other displays * Samsung G8 Neo 4k240hz * Samsung QN55QN95B 4k 120hz * Acer XV322QKKV 4k144hz * HP U27 4k Wireless 4k60hz * LG 27UD58B 4k60hz * LG 32UN650WA 4k60hz * LG Ultrafine 5k 5k60hz * AU Optronics B140HAN01.1 1080p 60hz eDP * AU Optronics B160UAN01.J 1920x1200 165hz eDP * AU Optronics B160QAN02.L 2560x1600 120hz eDP The patchset consists of the amd-staging-drm-next branch (Head commit - 685da0b7315c36fc61f356ed1592608f42aae5ff -> drm/amdgpu: optimize ACA log print) with new patches added on top of it. Tested on Ubuntu 24.04.1, on Wayland and X11, using KDE Plasma and Gnome. Tested-by: Daniel Wheeler Thank you, Dan Wheeler Sr. Technologist | AMD SW Display -- 1 Commerce Valley Dr E, Thornhill, ON L3T 7X6 amd.com -Original Message- From: Mohamed, Zaeem Sent: Friday, November 1, 2024 9:49 AM To: amd-gfx@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Siqueira, Rodrigo ; Mahfooz, Hamza ; Pillai, Aurabindo ; Li, Roman ; Lin, Wayne ; Chung, ChiaHsuan (Tom) ; Zuo, Jerry ; Mohamed, Zaeem ; Chiu, Solomon ; Wheeler, Daniel Subject: [PATCH 00/17] DC Patches October 28, 2024 This DC patchset brings improvements in multiple areas. In summary, we have: - Prune Invalid Modes for HDMI Output - SPL Cleanup - Fix brightness level not retained over reboot - Remove inaccessible registers from DMU diagnostics Cc: Daniel Wheeler Aric Cyr (1): drm/amd/display: 3.2.308 Aurabindo Pillai (1): drm/amd/display: parse umc_info or vram_info based on ASIC Ausef Yousof (3): Revert "drm/amd/display: Block UHBR Based On USB-C PD Cable ID" drm/amd/display: Remove hw w/a toggle if on DP2/HPO drm/amd/display: Remove otg w/a toggling on HPO interfaces Austin Zheng (1): drm/amd/display: Do Not Fallback To SW Cursor If HW Cursor Required Charlene Liu (1): drm/amd/display: avoid divided by zero Dominik Kaszewski (1): drm/amd/display: fix rxstatus_msg_sz type narrowing Fangzhi Zuo (1): drm/amd/display: Prune Invalid Modes For HDMI Output Ilya Bakoulin (1): drm/amd/display: Minimize wait for pending updates Kaitlyn Tse (1): drm/amd/display: Implement new backlight_level_params structure Nicholas Kazlauskas (1): drm/amd/display: Remove inaccessible registers from DMU diagnostics Samson Tam (2): drm/amd/display: fix asserts in SPL during bootup drm/amd/display: SPL cleanup Taimur Hassan (1): drm/amd/display: [FW Promotion] Release 0.0.241.0 Tom Chung (1): drm/amd/display: Fix brightness level not retained over reboot Wayne Lin (1): drm/amd/display: Don't write DP_MSTM_CTRL after LT .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 57 .../drm/amd/display/dc/bios/bios_parser2.c| 4 +- .../display/dc/clk_mgr/dcn35/dcn35_clk_mgr.c | 19 +++- drivers/gpu/drm/amd/display/dc/core/dc.c | 5 +- .../gpu/drm/amd/display/dc/core/dc_debug.c| 40 + .../drm/amd/display/dc/core/dc_link_exports.c | 5 +- .../gpu/drm/amd/display/dc/core/dc_stream.c | 10 ++- drivers/gpu/drm/amd/display/dc/dc.h | 6 +- .../gpu/drm/amd/display/dc/dc_spl_translate.c | 14 +-- drivers/gpu/drm/amd/display/dc/dc_types.h | 27 ++ .../amd/display/dc/hwss/dce110/dce110_hwseq.c | 6 +- .../amd/display/dc/h
RE: [PATCH 07/17] drm/amd/display: Fix brightness level not retained over reboot
[AMD Official Use Only - AMD Internal Distribution Only] Hi Mario, Do I need to re-send the patch to amd-gfx after the tags have been added or is sending upstream enough? Zaeem -Original Message- From: Limonciello, Mario Sent: Friday, November 1, 2024 11:43 AM To: Mohamed, Zaeem ; amd-gfx@lists.freedesktop.org Cc: Wentland, Harry ; Li, Sun peng (Leo) ; Siqueira, Rodrigo ; Mahfooz, Hamza ; Pillai, Aurabindo ; Li, Roman ; Lin, Wayne ; Chung, ChiaHsuan (Tom) ; Zuo, Jerry ; Chiu, Solomon ; Wheeler, Daniel ; mark.herber...@gmail.com Subject: Re: [PATCH 07/17] drm/amd/display: Fix brightness level not retained over reboot On 11/1/2024 08:49, Zaeem Mohamed wrote: > From: Tom Chung > > [Why] > During boot up and resume the DC layer will reset the panel brightness > to fix a flicker issue. > > It will cause the dm->actual_brightness is not the current panel > brightness level. (the dm->brightness is the correct panel level) > > [How] > Set the backlight level after do the set mode. > > Reviewed-by: Sun peng Li > Signed-off-by: Tom Chung > Signed-off-by: Zaeem Mohamed Some more tags, please explicitly add these while merging. Cc: sta...@vger.kernel.org Fixes: d9e865826c20 ("drm/amd/display: Simplify brightness initialization") Reported-by: Mark Herbert Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3655 > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index bbfc47f6595f..2599a99509de 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -9411,6 +9411,7 @@ static void amdgpu_dm_commit_streams(struct > drm_atomic_state *state, > bool mode_set_reset_required = false; > u32 i; > struct dc_commit_streams_params params = {dc_state->streams, > dc_state->stream_count}; > + bool set_backlight_level = false; > > /* Disable writeback */ > for_each_old_connector_in_state(state, connector, old_con_state, i) > { @@ -9530,6 +9531,7 @@ static void amdgpu_dm_commit_streams(struct > drm_atomic_state *state, > acrtc->hw_mode = new_crtc_state->mode; > crtc->hwmode = new_crtc_state->mode; > mode_set_reset_required = true; > + set_backlight_level = true; > } else if (modereset_required(new_crtc_state)) { > drm_dbg_atomic(dev, > "Atomic commit: RESET. crtc id > %d:[%p]\n", @@ -9581,6 > +9583,19 @@ static void amdgpu_dm_commit_streams(struct drm_atomic_state > *state, > acrtc->otg_inst = status->primary_otg_inst; > } > } > + > + /* During boot up and resume the DC layer will reset the panel > brightness > + * to fix a flicker issue. > + * It will cause the dm->actual_brightness is not the current panel > brightness > + * level. (the dm->brightness is the correct panel level) > + * So we set the backlight level with dm->brightness value after set > mode > + */ > + if (set_backlight_level) { > + for (i = 0; i < dm->num_of_edps; i++) { > + if (dm->backlight_dev[i]) > + amdgpu_dm_backlight_set_level(dm, i, > dm->brightness[i]); > + } > + } > } > > static void dm_set_writeback(struct amdgpu_display_manager *dm,
[PATCH v2 2/2] drm/display/dsc: MST DSC Interface Change
[why] Starting from dp2 where dsc passthrough is introduced, it is required to identify the dsc passthrough aux, apart from dsc decompression aux. Existing drm_dp_mst_port function that returns dsc_aux alone is not sufficient. [how] 1. Interface change in drm_dp_mst_dsc_aux_for_port, and dependency changes for each vendor. 2. Rename passthrough_aux with dsc_passthrough_aux to align with the name of dsc_aux. Signed-off-by: Fangzhi Zuo Signed-off-by: Wayne Lin --- .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 +-- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 30 drivers/gpu/drm/display/drm_dp_mst_topology.c | 34 +-- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- include/drm/display/drm_dp_mst_helper.h | 6 ++-- 7 files changed, 48 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c index db56b0aa5454..0da703f4ccac 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c @@ -1370,7 +1370,7 @@ static int dp_dsc_fec_support_show(struct seq_file *m, void *data) * enable DSC on the sink device or on MST branch * its connected to. */ - if (aconnector->dsc_aux) { + if (aconnector->mst_output_port->dsc_aux) { is_fec_supported = true; is_dsc_supported = true; } diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 069e0195e50a..2e5e490f9027 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -810,20 +810,20 @@ bool dm_helpers_dp_write_dsc_enable( uint8_t ret = 0; if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) { - if (!aconnector->dsc_aux) + if (!aconnector->mst_output_port->dsc_aux) return false; // apply w/a to synaptics if (needs_dsc_aux_workaround(aconnector->dc_link) && (aconnector->mst_downstream_port_present.byte & 0x7) != 0x3) return write_dsc_enable_synaptics_non_virtual_dpcd_mst( - aconnector->dsc_aux, stream, enable_dsc); + aconnector->mst_output_port->dsc_aux, stream, enable_dsc); port = aconnector->mst_output_port; if (enable) { - if (port->passthrough_aux) { - ret = drm_dp_dpcd_write(port->passthrough_aux, + if (port->dsc_passthrough_aux) { + ret = drm_dp_dpcd_write(port->dsc_passthrough_aux, DP_DSC_ENABLE, &enable_passthrough, 1); drm_dbg_dp(dev, @@ -831,24 +831,24 @@ bool dm_helpers_dp_write_dsc_enable( ret); } - ret = drm_dp_dpcd_write(aconnector->dsc_aux, + ret = drm_dp_dpcd_write(aconnector->mst_output_port->dsc_aux, DP_DSC_ENABLE, &enable_dsc, 1); drm_dbg_dp(dev, "MST_DSC Sent DSC decoding enable to %s port, ret = %u\n", - (port->passthrough_aux) ? "remote RX" : + (port->dsc_passthrough_aux) ? "remote RX" : "virtual dpcd", ret); } else { - ret = drm_dp_dpcd_write(aconnector->dsc_aux, + ret = drm_dp_dpcd_write(aconnector->mst_output_port->dsc_aux, DP_DSC_ENABLE, &enable_dsc, 1); drm_dbg_dp(dev, "MST_DSC Sent DSC decoding disable to %s port, ret = %u\n", - (port->passthrough_aux) ? "remote RX" : + (port->dsc_passthrough_aux) ? "remote RX" : "virtual dpcd", ret); - if (port->passthrough_aux) { - ret = drm_dp_dpcd_write(port->passthrough_aux, + if (port->dsc_passthrough_aux) { + ret = drm_dp_dpcd_write(port->dsc_passthro
[PATCH v2 0/2] Refactor MST DSC Determination Policy
The patch series is to refactor existing dsc determination policy for dsc decompression and dsc passthrough given a mst output port. Original routine was written based on different peer device types which is not accurate and shows difficulty when expanding support of products that do not fully comply with DP specs. To make the routine more accurate and generic, the series includes below changes: 1. Refactor MST DSC determination policy solely based on topology connection status and dsc dpcd capability info. 2. Dependency changes required for each vendor due to interface change. Fangzhi Zuo (2): drm/display/dsc: Refactor DRM MST DSC Determination Policy drm/display/dsc: MST DSC Interface Change .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 20 +- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 30 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 258 -- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 +- include/drm/display/drm_dp_mst_helper.h | 9 +- 7 files changed, 145 insertions(+), 179 deletions(-) -- 2.43.0
Re: [PATCH v8 2/4] drm/doc: Document device wedged event
On Tue, Oct 29, 2024 at 10:51:34AM +0100, Christian König wrote: > Am 25.10.24 um 10:48 schrieb Raag Jadav: > > Add documentation for device wedged event in a new 'Device wedging' > > chapter. The describes basic definitions and consumer expectations > > along with an example. > > > > v8: Improve documentation (Christian, Rodrigo) > > > > Signed-off-by: Raag Jadav > > --- > > Documentation/gpu/drm-uapi.rst | 75 ++ > > 1 file changed, 75 insertions(+) > > > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > > index 370d820be248..11a7446233b5 100644 > > --- a/Documentation/gpu/drm-uapi.rst > > +++ b/Documentation/gpu/drm-uapi.rst > > @@ -362,6 +362,81 @@ the first place. DRM devices should make use of > > devcoredump to store relevant > > information about the reset, so this information can be added to user bug > > reports. > > +Device wedging > > +== > > + > > +Drivers can optionally make use of device wedged event (implemented as > > +drm_dev_wedged_event() in DRM subsystem) which notifies userspace of wedged > > +(hanged/unusable) state of the DRM device through a uevent. This is useful > > +especially in cases where the device is no longer operating as expected > > even > > +after a reset and has become unrecoverable from driver context. Purpose of > > +this implementation is to provide drivers a generic way to recover with the > > +help of userspace intervention without taking any drastic measures in the > > +driver. > > + > > +A 'wedged' device is basically a dead device that needs attention. The > > +uevent is the notification that is sent to userspace along with a hint > > about > > +what could possibly be attempted to recover the device and bring it back to > > +usable state. Different drivers may have different ideas of a 'wedged' > > device > > +depending on their hardware implementation, and hence the vendor agnostic > > +nature of the event. It is up to the drivers to decide when they see the > > need > > +for recovery and how they want to recover from the available methods. > > + > > +Recovery > > + > > + > > +Current implementation defines two recovery methods, out of which, drivers > > +can use any one, both or none. Method(s) of choice will be sent in the > > uevent > > +environment as ``WEDGED=[,]`` in order of less to more > > side > > +effects. If driver is unsure about recovery or method is unknown (like > > reboot, > > +firmware flashing, hardware replacement or any other procedure which can't > > be > > +attempted on the fly), ``WEDGED=none`` will be sent instead. > > + > > +It is the responsibility of the driver to perform required cleanups (like > > +disabling system memory access or signalling dma_fences) and prepare itself > > +for the recovery before sending the event. > > That needs to be more like a warning and should have a bit more text. Maybe > even a separate section. > > Something like this maybe: > > Prerequisites > -- > > Drivers needs to make sure that the device won't harm the system as a > whole by keeping it in a wedged state. Necessary actions must include > disabling DMA to system memory as well as communication channels > with other devices. > Further drivers must ensure that all dma_fences are signaled and other > state the core kernel might depend on are cleaned up. > All ongoing waiting for hardware state changes must be aborted and > new accesses to the hardware sufficiently blocked > > > I'm not a native speaker of English, so feel free to re-phrase that. But the Neither am I, but will try my best. > general approach should be like don't do this before you have made sure a, b > and c. Sure, thanks. > > Once the event is sent, driver > > +should block all IOCTLs with an error code. > > Better define which error code that should be. I think -ENODEV similar to a > hotplug case would be appropriate. Why not leave it to the drivers to decide depending on the type of failure? > > This will signify the reason for > > +wegeding which can be reported to the application if needed. > > + > > +Userspace consumers can parse this event and attempt recovery as per below > > +expectations. > > + > > +=== == > > +Recovery method Consumer expectations > > +=== == > > +rebind unbind + rebind driver > > +bus-reset unbind + reset bus device + rebind > > +noneadmin/user policy > > +=== == > > + > > +Example for rebind > > +~~ > > + > > +Udev rule:: > > + > > +SUBSYSTEM=="drm", ENV{WEDGED}=="rebind", DEVPATH=="*/drm/card[0-9]", > > +RUN+="/path/to/rebind.sh $env{DEVPATH}" > > + > > +Recovery script:: > > + > > +#!/bin/sh > > + > > +DEVPATH=$(readlink -f /sys/$1/device) > > +DEVICE=$(basename $DEVPATH) > > +DRIVER=$(readlin
Re: [PATCH v2] amdgpu: prevent NULL pointer dereference if ATIF is not supported
On 31/10/2024 20:37, Mario Limonciello wrote: On 10/31/2024 10:28, Antonio Quartulli wrote: acpi_evaluate_object() may return AE_NOT_FOUND (failure), which would result in dereferencing buffer.pointer (obj) while being NULL. Although this case may be unrealistic for the current code, it is still better to protect against possible bugs. Bail out also when status is AE_NOT_FOUND. This fixes 1 FORWARD_NULL issue reported by Coverity Report: CID 1600951: Null pointer dereferences (FORWARD_NULL) Signed-off-by: Antonio Quartulli Can you please dig up the right Fixes: tag? Fixes: c9b7c809b89f ("drm/amd: Guard against bad data for ATIF ACPI method") Your commit :) Should I send v3 with the Fixes tag in it? Interestingly, this pattern of checking for AE_NOT_FOUND is shared by other functions, however, they don't try to dereference the pointer to the buffer before the return statement (which caused the Coverity report). It's the caller that checks if the return value is NULL or not. For this function it was the same, until you added this extra check on obj->type, without checking if obj was NULL or not. If we want to keep the original pattern and continue checking for AE_NOT_FOUND, we could rather do: - if (obj->type != ACPI_TYPE_BUFFER) { + if (obj && obj->type != ACPI_TYPE_BUFFER) { But this feel more like "bike shed color picking" than anything else :) Anyway, up to you Mario, I am open to change the patch again if the latter pattern is more preferable. Regards, Besides that, LGTM. Reviewed-by: Mario Limonciello --- drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/ drm/amd/amdgpu/amdgpu_acpi.c index cce85389427f..b8d4e07d2043 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -172,8 +172,8 @@ static union acpi_object *amdgpu_atif_call(struct amdgpu_atif *atif, &buffer); obj = (union acpi_object *)buffer.pointer; - /* Fail if calling the method fails and ATIF is supported */ - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) { + /* Fail if calling the method fails */ + if (ACPI_FAILURE(status)) { DRM_DEBUG_DRIVER("failed to evaluate ATIF got %s\n", acpi_format_exception(status)); kfree(obj); -- Antonio Quartulli CEO and Co-Founder Mandelbit Srl https://www.mandelbit.com
RE: [PATCH] drm/amdgpu: Add compatible NPS mode info
[AMD Official Use Only - AMD Internal Distribution Only] Reviewed-by: Asad Kamal Thanks & Regards Asad -Original Message- From: amd-gfx On Behalf Of Lijo Lazar Sent: Wednesday, October 30, 2024 2:00 PM To: amd-gfx@lists.freedesktop.org Cc: Zhang, Hawking ; Deucher, Alexander ; Kamal, Asad Subject: [PATCH] drm/amdgpu: Add compatible NPS mode info Populate the compatible NPS modes also for providing partition configuration details through sysfs. Signed-off-by: Lijo Lazar --- drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.h| 1 + drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 11 +++ 2 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.h index 7ac89d78a5bf..b63f53242c57 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.h @@ -77,6 +77,7 @@ struct amdgpu_xcp_cfg { u8 num_res; struct amdgpu_xcp_mgr *xcp_mgr; struct kobject kobj; + u16 compatible_nps_modes; }; struct amdgpu_xcp_ip_funcs { diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c index 890976b7ce77..fea0d2d0 100644 --- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c +++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c @@ -455,6 +455,7 @@ static int aqua_vanjaram_get_xcp_res_info(struct amdgpu_xcp_mgr *xcp_mgr, int max_res[AMDGPU_XCP_RES_MAX] = {}; bool res_lt_xcp; int num_xcp, i; + u16 nps_modes; if (!(xcp_mgr->supp_xcp_modes & BIT(mode))) return -EINVAL; @@ -467,23 +468,33 @@ static int aqua_vanjaram_get_xcp_res_info(struct amdgpu_xcp_mgr *xcp_mgr, switch (mode) { case AMDGPU_SPX_PARTITION_MODE: num_xcp = 1; + nps_modes = BIT(AMDGPU_NPS1_PARTITION_MODE); break; case AMDGPU_DPX_PARTITION_MODE: num_xcp = 2; + nps_modes = BIT(AMDGPU_NPS1_PARTITION_MODE); break; case AMDGPU_TPX_PARTITION_MODE: num_xcp = 3; + nps_modes = BIT(AMDGPU_NPS1_PARTITION_MODE) | + BIT(AMDGPU_NPS4_PARTITION_MODE); break; case AMDGPU_QPX_PARTITION_MODE: num_xcp = 4; + nps_modes = BIT(AMDGPU_NPS1_PARTITION_MODE) | + BIT(AMDGPU_NPS4_PARTITION_MODE); break; case AMDGPU_CPX_PARTITION_MODE: num_xcp = NUM_XCC(adev->gfx.xcc_mask); + nps_modes = BIT(AMDGPU_NPS1_PARTITION_MODE) | + BIT(AMDGPU_NPS4_PARTITION_MODE); break; default: return -EINVAL; } + xcp_cfg->compatible_nps_modes = + (adev->gmc.supported_nps_modes & nps_modes); xcp_cfg->num_res = ARRAY_SIZE(max_res); for (i = 0; i < xcp_cfg->num_res; i++) { -- 2.25.1