From: Wayne Lin <Wayne.Lin@xxxxxxx> [Why] Variables relates to secure display are spreading out within struct amdgpu_display_manager. [How] Encapsulate relevant variables into struct secure_display_context and adjust relevant affected codes. Reviewed-by: HaoPing Liu <haoping.liu@xxxxxxx> Signed-off-by: Wayne Lin <Wayne.Lin@xxxxxxx> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@xxxxxxx> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 16 +-- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 12 +- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 109 +++++++++--------- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 16 ++- 4 files changed, 81 insertions(+), 72 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 85f21db6ef24..f5aff4dfbe6f 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2152,8 +2152,8 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) } #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) - adev->dm.secure_display_ctxs = amdgpu_dm_crtc_secure_display_create_contexts(adev); - if (!adev->dm.secure_display_ctxs) + amdgpu_dm_crtc_secure_display_create_contexts(adev); + if (!adev->dm.secure_display_ctx.crtc_ctx) DRM_ERROR("amdgpu: failed to initialize secure display contexts.\n"); #endif @@ -2197,15 +2197,15 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev) amdgpu_dm_destroy_drm_device(&adev->dm); #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) - if (adev->dm.secure_display_ctxs) { + if (adev->dm.secure_display_ctx.crtc_ctx) { for (i = 0; i < adev->mode_info.num_crtc; i++) { - if (adev->dm.secure_display_ctxs[i].crtc) { - flush_work(&adev->dm.secure_display_ctxs[i].notify_ta_work); - flush_work(&adev->dm.secure_display_ctxs[i].forward_roi_work); + if (adev->dm.secure_display_ctx.crtc_ctx[i].crtc) { + flush_work(&adev->dm.secure_display_ctx.crtc_ctx[i].notify_ta_work); + flush_work(&adev->dm.secure_display_ctx.crtc_ctx[i].forward_roi_work); } } - kfree(adev->dm.secure_display_ctxs); - adev->dm.secure_display_ctxs = NULL; + kfree(adev->dm.secure_display_ctx.crtc_ctx); + adev->dm.secure_display_ctx.crtc_ctx = NULL; } #endif if (adev->dm.hdcp_workqueue) { diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h index cbd7a1cb34af..e46e1365fe91 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h @@ -541,16 +541,12 @@ struct amdgpu_display_manager { #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) /** - * @secure_display_ctxs: + * @secure_display_ctx: * - * Store the ROI information and the work_struct to command dmub and psp for - * all crtcs. + * Store secure display relevant info. e.g. the ROI information + * , the work_struct to command dmub, etc. */ - struct secure_display_context *secure_display_ctxs; - - bool secure_display_phy_mapping_updated; - int phy_id_mapping_cnt; - struct phy_id_mapping phy_id_mapping[AMDGPU_DM_MAX_CRTC]; + struct secure_display_context secure_display_ctx; #endif /** * @hpd_rx_offload_wq: diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index 066054caa4c7..309c7999faa6 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -93,7 +93,7 @@ static void update_phy_id_mapping(struct amdgpu_device *adev) struct drm_connector_list_iter iter; uint8_t idx = 0, idx_2 = 0, connector_cnt = 0; - dm->secure_display_phy_mapping_updated = false; + dm->secure_display_ctx.phy_mapping_updated = false; mutex_lock(&ddev->mode_config.mutex); drm_connector_list_iter_begin(ddev, &iter); @@ -194,27 +194,27 @@ static void update_phy_id_mapping(struct amdgpu_device *adev) } } - /* Complete sorting. Assign relavant result to dm->phy_id_mapping[]*/ - memset(dm->phy_id_mapping, 0, sizeof(dm->phy_id_mapping)); + /* Complete sorting. Assign relavant result to dm->secure_display_ctx.phy_id_mapping[]*/ + memset(dm->secure_display_ctx.phy_id_mapping, 0, sizeof(dm->secure_display_ctx.phy_id_mapping)); for (idx = 0; idx < connector_cnt; idx++) { aconnector = sort_connector[idx]; - dm->phy_id_mapping[idx].assigned = true; - dm->phy_id_mapping[idx].is_mst = false; - dm->phy_id_mapping[idx].enc_hw_inst = aconnector->dc_link->link_enc_hw_inst; + dm->secure_display_ctx.phy_id_mapping[idx].assigned = true; + dm->secure_display_ctx.phy_id_mapping[idx].is_mst = false; + dm->secure_display_ctx.phy_id_mapping[idx].enc_hw_inst = aconnector->dc_link->link_enc_hw_inst; if (sort_connector[idx]->mst_root) { - dm->phy_id_mapping[idx].is_mst = true; - dm->phy_id_mapping[idx].lct = aconnector->mst_output_port->parent->lct; - dm->phy_id_mapping[idx].port_num = aconnector->mst_output_port->port_num; - memcpy(dm->phy_id_mapping[idx].rad, aconnector->mst_output_port->parent->rad, - sizeof(aconnector->mst_output_port->parent->rad)); + dm->secure_display_ctx.phy_id_mapping[idx].is_mst = true; + dm->secure_display_ctx.phy_id_mapping[idx].lct = aconnector->mst_output_port->parent->lct; + dm->secure_display_ctx.phy_id_mapping[idx].port_num = aconnector->mst_output_port->port_num; + memcpy(dm->secure_display_ctx.phy_id_mapping[idx].rad, + aconnector->mst_output_port->parent->rad, sizeof(aconnector->mst_output_port->parent->rad)); } } mutex_unlock(&ddev->mode_config.mutex); - dm->phy_id_mapping_cnt = connector_cnt; - dm->secure_display_phy_mapping_updated = true; + dm->secure_display_ctx.phy_id_mapping_cnt = connector_cnt; + dm->secure_display_ctx.phy_mapping_updated = true; } static bool get_phy_id(struct amdgpu_display_manager *dm, @@ -227,19 +227,20 @@ static bool get_phy_id(struct amdgpu_display_manager *dm, * Assume secure display start after all connectors are probed. The connection * config is static as well */ - if (!dm->secure_display_phy_mapping_updated) { + if (!dm->secure_display_ctx.phy_mapping_updated) { DRM_WARN("%s Should update the phy id table before get it's value", __func__); return false; } - for (idx = 0; idx < dm->phy_id_mapping_cnt; idx++) { - if (!dm->phy_id_mapping[idx].assigned) { + for (idx = 0; idx < dm->secure_display_ctx.phy_id_mapping_cnt; idx++) { + if (!dm->secure_display_ctx.phy_id_mapping[idx].assigned) { DRM_ERROR("phy_id_mapping[%d] should be assigned", idx); return false; } - if (aconnector->dc_link->link_enc_hw_inst == dm->phy_id_mapping[idx].enc_hw_inst) { - if (!dm->phy_id_mapping[idx].is_mst) { + if (aconnector->dc_link->link_enc_hw_inst == + dm->secure_display_ctx.phy_id_mapping[idx].enc_hw_inst) { + if (!dm->secure_display_ctx.phy_id_mapping[idx].is_mst) { found = true; goto out; } else { @@ -254,8 +255,8 @@ static bool get_phy_id(struct amdgpu_display_manager *dm, DRM_WARN("%s pass in a stale mst connector", __func__); } - if (aconnector->mst_output_port->parent->lct == dm->phy_id_mapping[idx].lct && - aconnector->mst_output_port->port_num == dm->phy_id_mapping[idx].port_num) { + if (aconnector->mst_output_port->parent->lct == dm->secure_display_ctx.phy_id_mapping[idx].lct && + aconnector->mst_output_port->port_num == dm->secure_display_ctx.phy_id_mapping[idx].port_num) { if (aconnector->mst_output_port->parent->lct == 1) { found = true; goto out; @@ -264,7 +265,7 @@ static bool get_phy_id(struct amdgpu_display_manager *dm, for (idx_2 = 0; idx_2 < aconnector->mst_output_port->parent->lct - 1; idx_2++) { int shift = (idx_2 % 2) ? 0 : 4; int port_num = (aconnector->mst_output_port->parent->rad[idx_2 / 2] >> shift) & 0xf; - int port_num2 = (dm->phy_id_mapping[idx].rad[idx_2 / 2] >> shift) & 0xf; + int port_num2 = (dm->secure_display_ctx.phy_id_mapping[idx].rad[idx_2 / 2] >> shift) & 0xf; if (port_num != port_num2) break; @@ -318,8 +319,8 @@ static void amdgpu_dm_set_crc_window_default(struct drm_crtc *crtc, struct dc_st /* Disable secure_display if it was enabled */ if (was_activated) { /* stop ROI update on this crtc */ - flush_work(&dm->secure_display_ctxs[crtc->index].notify_ta_work); - flush_work(&dm->secure_display_ctxs[crtc->index].forward_roi_work); + flush_work(&dm->secure_display_ctx.crtc_ctx[crtc->index].notify_ta_work); + flush_work(&dm->secure_display_ctx.crtc_ctx[crtc->index].forward_roi_work); aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; @@ -332,7 +333,7 @@ static void amdgpu_dm_set_crc_window_default(struct drm_crtc *crtc, struct dc_st static void amdgpu_dm_crtc_notify_ta_to_read(struct work_struct *work) { - struct secure_display_context *secure_display_ctx; + struct secure_display_crtc_context *crtc_ctx; struct psp_context *psp; struct ta_securedisplay_cmd *securedisplay_cmd; struct drm_crtc *crtc; @@ -342,8 +343,8 @@ static void amdgpu_dm_crtc_notify_ta_to_read(struct work_struct *work) struct amdgpu_display_manager *dm; int ret; - secure_display_ctx = container_of(work, struct secure_display_context, notify_ta_work); - crtc = secure_display_ctx->crtc; + crtc_ctx = container_of(work, struct secure_display_crtc_context, notify_ta_work); + crtc = crtc_ctx->crtc; if (!crtc) return; @@ -393,15 +394,15 @@ static void amdgpu_dm_crtc_notify_ta_to_read(struct work_struct *work) static void amdgpu_dm_forward_crc_window(struct work_struct *work) { - struct secure_display_context *secure_display_ctx; + struct secure_display_crtc_context *crtc_ctx; struct amdgpu_display_manager *dm; struct drm_crtc *crtc; struct dc_stream_state *stream; struct amdgpu_dm_connector *aconnector; uint8_t phy_id; - secure_display_ctx = container_of(work, struct secure_display_context, forward_roi_work); - crtc = secure_display_ctx->crtc; + crtc_ctx = container_of(work, struct secure_display_crtc_context, forward_roi_work); + crtc = crtc_ctx->crtc; if (!crtc) return; @@ -422,7 +423,7 @@ amdgpu_dm_forward_crc_window(struct work_struct *work) mutex_unlock(&crtc->dev->mode_config.mutex); mutex_lock(&dm->dc_lock); - dc_stream_forward_crc_window(stream, &secure_display_ctx->rect, + dc_stream_forward_crc_window(stream, &crtc_ctx->rect, phy_id, false); mutex_unlock(&dm->dc_lock); } @@ -656,7 +657,7 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) /* Initialize phy id mapping table for secure display*/ - if (!dm->secure_display_phy_mapping_updated) + if (!dm->secure_display_ctx.phy_mapping_updated) update_phy_id_mapping(adev); #endif @@ -730,7 +731,7 @@ void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc) enum amdgpu_dm_pipe_crc_source cur_crc_src; struct amdgpu_crtc *acrtc = NULL; struct amdgpu_device *adev = NULL; - struct secure_display_context *secure_display_ctx = NULL; + struct secure_display_crtc_context *crtc_ctx = NULL; unsigned long flags1; if (crtc == NULL) @@ -756,23 +757,23 @@ void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc) goto cleanup; } - secure_display_ctx = &adev->dm.secure_display_ctxs[acrtc->crtc_id]; - if (WARN_ON(secure_display_ctx->crtc != crtc)) { - /* We have set the crtc when creating secure_display_context, + crtc_ctx = &adev->dm.secure_display_ctx.crtc_ctx[acrtc->crtc_id]; + if (WARN_ON(crtc_ctx->crtc != crtc)) { + /* We have set the crtc when creating secure_display_crtc_context, * don't expect it to be changed here. */ - secure_display_ctx->crtc = crtc; + crtc_ctx->crtc = crtc; } if (acrtc->dm_irq_params.window_param.update_win) { /* prepare work for dmub to update ROI */ - secure_display_ctx->rect.x = acrtc->dm_irq_params.window_param.x_start; - secure_display_ctx->rect.y = acrtc->dm_irq_params.window_param.y_start; - secure_display_ctx->rect.width = acrtc->dm_irq_params.window_param.x_end - + crtc_ctx->rect.x = acrtc->dm_irq_params.window_param.x_start; + crtc_ctx->rect.y = acrtc->dm_irq_params.window_param.y_start; + crtc_ctx->rect.width = acrtc->dm_irq_params.window_param.x_end - acrtc->dm_irq_params.window_param.x_start; - secure_display_ctx->rect.height = acrtc->dm_irq_params.window_param.y_end - + crtc_ctx->rect.height = acrtc->dm_irq_params.window_param.y_end - acrtc->dm_irq_params.window_param.y_start; - schedule_work(&secure_display_ctx->forward_roi_work); + schedule_work(&crtc_ctx->forward_roi_work); acrtc->dm_irq_params.window_param.update_win = false; @@ -785,32 +786,34 @@ void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc) } else { /* prepare work for psp to read ROI/CRC and send to I2C */ - schedule_work(&secure_display_ctx->notify_ta_work); + schedule_work(&crtc_ctx->notify_ta_work); } cleanup: spin_unlock_irqrestore(&drm_dev->event_lock, flags1); } -struct secure_display_context * -amdgpu_dm_crtc_secure_display_create_contexts(struct amdgpu_device *adev) +void amdgpu_dm_crtc_secure_display_create_contexts(struct amdgpu_device *adev) { - struct secure_display_context *secure_display_ctxs = NULL; + struct secure_display_crtc_context *crtc_ctx = NULL; int i; - secure_display_ctxs = kcalloc(adev->mode_info.num_crtc, - sizeof(struct secure_display_context), + crtc_ctx = kcalloc(adev->mode_info.num_crtc, + sizeof(struct secure_display_crtc_context), GFP_KERNEL); - if (!secure_display_ctxs) - return NULL; + if (!crtc_ctx) { + adev->dm.secure_display_ctx.crtc_ctx = NULL; + return; + } for (i = 0; i < adev->mode_info.num_crtc; i++) { - INIT_WORK(&secure_display_ctxs[i].forward_roi_work, amdgpu_dm_forward_crc_window); - INIT_WORK(&secure_display_ctxs[i].notify_ta_work, amdgpu_dm_crtc_notify_ta_to_read); - secure_display_ctxs[i].crtc = &adev->mode_info.crtcs[i]->base; + INIT_WORK(&crtc_ctx[i].forward_roi_work, amdgpu_dm_forward_crc_window); + INIT_WORK(&crtc_ctx[i].notify_ta_work, amdgpu_dm_crtc_notify_ta_to_read); + crtc_ctx[i].crtc = &adev->mode_info.crtcs[i]->base; } - return secure_display_ctxs; + adev->dm.secure_display_ctx.crtc_ctx = crtc_ctx; + return; } #endif diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h index 82afb551632b..4ec2510bc312 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h @@ -40,6 +40,8 @@ enum amdgpu_dm_pipe_crc_source { }; #ifdef CONFIG_DRM_AMD_SECURE_DISPLAY +#define MAX_CRTC 6 + struct phy_id_mapping { bool assigned; bool is_mst; @@ -62,7 +64,7 @@ struct crc_window_param { int skip_frame_cnt; }; -struct secure_display_context { +struct secure_display_crtc_context { /* work to notify PSP TA*/ struct work_struct notify_ta_work; @@ -74,6 +76,15 @@ struct secure_display_context { /* Region of Interest (ROI) */ struct rect rect; }; + +struct secure_display_context { + + struct secure_display_crtc_context *crtc_ctx; + + bool phy_mapping_updated; + int phy_id_mapping_cnt; + struct phy_id_mapping phy_id_mapping[MAX_CRTC]; +}; #endif static inline bool amdgpu_dm_is_valid_crc_source(enum amdgpu_dm_pipe_crc_source source) @@ -104,8 +115,7 @@ void amdgpu_dm_crtc_handle_crc_irq(struct drm_crtc *crtc); #ifdef CONFIG_DRM_AMD_SECURE_DISPLAY bool amdgpu_dm_crc_window_is_activated(struct drm_crtc *crtc); void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc); -struct secure_display_context *amdgpu_dm_crtc_secure_display_create_contexts( - struct amdgpu_device *adev); +void amdgpu_dm_crtc_secure_display_create_contexts(struct amdgpu_device *adev); #else #define amdgpu_dm_crc_window_is_activated(x) #define amdgpu_dm_crtc_handle_crc_window_irq(x) -- 2.47.1