[AMD Public Use] Thanks Siqueira. Comments below. > -----Original Message----- > From: Siqueira, Rodrigo <Rodrigo.Siqueira@xxxxxxx> > Sent: Friday, January 8, 2021 4:53 AM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: Lin, Wayne <Wayne.Lin@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Wentland, Harry > <Harry.Wentland@xxxxxxx>; Li, Roman <Roman.Li@xxxxxxx>; R, Bindu <Bindu.R@xxxxxxx>; Daniel Vetter <daniel@xxxxxxxx> > Subject: [PATCH 3/3] Revert "drm/amd/display: Expose new CRC window property" > > This reverts commit 110d586ba77ed573eb7464ca69b6490ec0b70c5f. > > Cc: Wayne Lin <Wayne.Lin@xxxxxxx> > Cc: Alexander Deucher <Alexander.Deucher@xxxxxxx> > Cc: Harry Wentland <Harry.Wentland@xxxxxxx> > Cc: Roman Li <Roman.Li@xxxxxxx> > Cc: Bindu R <Bindu.R@xxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@xxxxxxx> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 142 +----------------- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | > 19 --- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 56 +------ > .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 3 - > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 + > 5 files changed, 12 insertions(+), 210 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 a06554745238..0515ed0d125c 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -938,41 +938,6 @@ static void mmhub_read_system_context(struct amdgpu_device *adev, struct dc_phy_ } #endif > > -#ifdef CONFIG_DEBUG_FS > -static int create_crtc_crc_properties(struct amdgpu_display_manager *dm) -{ > -dm->crc_win_x_start_property = > -drm_property_create_range(adev_to_drm(dm->adev), > - DRM_MODE_PROP_ATOMIC, > - "AMD_CRC_WIN_X_START", 0, U16_MAX); > -if (!dm->crc_win_x_start_property) > -return -ENOMEM; > - > -dm->crc_win_y_start_property = > -drm_property_create_range(adev_to_drm(dm->adev), > - DRM_MODE_PROP_ATOMIC, > - "AMD_CRC_WIN_Y_START", 0, U16_MAX); > -if (!dm->crc_win_y_start_property) > -return -ENOMEM; > - > -dm->crc_win_x_end_property = > -drm_property_create_range(adev_to_drm(dm->adev), > - DRM_MODE_PROP_ATOMIC, > - "AMD_CRC_WIN_X_END", 0, U16_MAX); > -if (!dm->crc_win_x_end_property) > -return -ENOMEM; > - > -dm->crc_win_y_end_property = > -drm_property_create_range(adev_to_drm(dm->adev), > - DRM_MODE_PROP_ATOMIC, > - "AMD_CRC_WIN_Y_END", 0, U16_MAX); > -if (!dm->crc_win_y_end_property) > -return -ENOMEM; > - > -return 0; > -} > -#endif > - > static int amdgpu_dm_init(struct amdgpu_device *adev) { > struct dc_init_data init_data; > @@ -1119,10 +1084,6 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) > > dc_init_callbacks(adev->dm.dc, &init_params); > } > -#endif > -#ifdef CONFIG_DEBUG_FS > -if (create_crtc_crc_properties(&adev->dm)) > -DRM_ERROR("amdgpu: failed to create crc property.\n"); > #endif > if (amdgpu_dm_initialize_drm_device(adev)) { > DRM_ERROR( > @@ -5456,64 +5417,12 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) > state->crc_src = cur->crc_src; > state->cm_has_degamma = cur->cm_has_degamma; > state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb; -#ifdef CONFIG_DEBUG_FS > -state->crc_window = cur->crc_window; > -#endif > + > /* TODO Duplicate dc_stream after objects are stream object is flattened */ > > return &state->base; > } > > -#ifdef CONFIG_DEBUG_FS > -static int amdgpu_dm_crtc_atomic_set_property(struct drm_crtc *crtc, > - struct drm_crtc_state *crtc_state, > - struct drm_property *property, > - uint64_t val) > -{ > -struct drm_device *dev = crtc->dev; > -struct amdgpu_device *adev = drm_to_adev(dev); > -struct dm_crtc_state *dm_new_state = > -to_dm_crtc_state(crtc_state); > - > -if (property == adev->dm.crc_win_x_start_property) > -dm_new_state->crc_window.x_start = val; > -else if (property == adev->dm.crc_win_y_start_property) > -dm_new_state->crc_window.y_start = val; > -else if (property == adev->dm.crc_win_x_end_property) > -dm_new_state->crc_window.x_end = val; > -else if (property == adev->dm.crc_win_y_end_property) > -dm_new_state->crc_window.y_end = val; > -else > -return -EINVAL; > - > -return 0; > -} > - > -static int amdgpu_dm_crtc_atomic_get_property(struct drm_crtc *crtc, > - const struct drm_crtc_state *state, > - struct drm_property *property, > - uint64_t *val) > -{ > -struct drm_device *dev = crtc->dev; > -struct amdgpu_device *adev = drm_to_adev(dev); > -struct dm_crtc_state *dm_state = > -to_dm_crtc_state(state); > - > -if (property == adev->dm.crc_win_x_start_property) > -*val = dm_state->crc_window.x_start; > -else if (property == adev->dm.crc_win_y_start_property) > -*val = dm_state->crc_window.y_start; > -else if (property == adev->dm.crc_win_x_end_property) > -*val = dm_state->crc_window.x_end; > -else if (property == adev->dm.crc_win_y_end_property) > -*val = dm_state->crc_window.y_end; > -else > -return -EINVAL; > - > -return 0; > -} > -#endif > - > static inline int dm_set_vupdate_irq(struct drm_crtc *crtc, bool enable) { > enum dc_irq_source irq_source; > @@ -5599,10 +5508,6 @@ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { > .enable_vblank = dm_enable_vblank, > .disable_vblank = dm_disable_vblank, > .get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp, > -#ifdef CONFIG_DEBUG_FS > -.atomic_set_property = amdgpu_dm_crtc_atomic_set_property, > -.atomic_get_property = amdgpu_dm_crtc_atomic_get_property, > -#endif > }; > > static enum drm_connector_status > @@ -6829,25 +6734,6 @@ static int amdgpu_dm_plane_init(struct amdgpu_display_manager *dm, > return 0; > } > > -#ifdef CONFIG_DEBUG_FS > -static void attach_crtc_crc_properties(struct amdgpu_display_manager *dm, > -struct amdgpu_crtc *acrtc) > -{ > -drm_object_attach_property(&acrtc->base.base, > - dm->crc_win_x_start_property, > - 0); > -drm_object_attach_property(&acrtc->base.base, > - dm->crc_win_y_start_property, > - 0); > -drm_object_attach_property(&acrtc->base.base, > - dm->crc_win_x_end_property, > - 0); > -drm_object_attach_property(&acrtc->base.base, > - dm->crc_win_y_end_property, > - 0); > -} > -#endif > - > static int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm, > struct drm_plane *plane, > uint32_t crtc_index) > @@ -6895,9 +6781,7 @@ static int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm, > drm_crtc_enable_color_mgmt(&acrtc->base, MAX_COLOR_LUT_ENTRIES, > true, MAX_COLOR_LUT_ENTRIES); > drm_mode_crtc_set_gamma_size(&acrtc->base, MAX_COLOR_LEGACY_LUT_ENTRIES); -#ifdef CONFIG_DEBUG_FS > -attach_crtc_crc_properties(dm, acrtc); > -#endif > + > return 0; > > fail: > @@ -8538,7 +8422,6 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) > */ > for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { > struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > -bool configure_crc = false; > > dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); > > @@ -8548,30 +8431,21 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) > dc_stream_retain(dm_new_crtc_state->stream); > acrtc->dm_irq_params.stream = dm_new_crtc_state->stream; > manage_dm_interrupts(adev, acrtc, true); > -} > + > #ifdef CONFIG_DEBUG_FS > -if (new_crtc_state->active && > -amdgpu_dm_is_valid_crc_source(dm_new_crtc_state->crc_src)) { > /** > * Frontend may have changed so reapply the CRC capture > * settings for the stream. > */ > dm_new_crtc_state = to_dm_crtc_state(new_crtc_state); > -dm_old_crtc_state = to_dm_crtc_state(old_crtc_state); > - > -if (amdgpu_dm_crc_window_is_default(dm_new_crtc_state)) { > -if (!old_crtc_state->active || drm_atomic_crtc_needs_modeset(new_crtc_state)) > -configure_crc = true; > -} else { > -if (amdgpu_dm_crc_window_changed(dm_new_crtc_state, dm_old_crtc_state)) > -configure_crc = true; > -} > > -if (configure_crc) > +if (amdgpu_dm_is_valid_crc_source(dm_new_crtc_state->crc_src)) { > amdgpu_dm_crtc_configure_crc_source( > -crtc, dm_new_crtc_state, dm_new_crtc_state->crc_src); > -} > +crtc, dm_new_crtc_state, > +dm_new_crtc_state->crc_src); > +} > #endif > +} > } > > for_each_new_crtc_in_state(state, crtc, new_crtc_state, j) 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 ef394e941d9d..9d22d62ac2d1 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h > @@ -343,13 +343,6 @@ struct amdgpu_display_manager { > */ > uint32_t active_vblank_irq_count; > > -#ifdef CONFIG_DEBUG_FS > -/* set the crc calculation window*/ > -struct drm_property *crc_win_x_start_property; > -struct drm_property *crc_win_y_start_property; > -struct drm_property *crc_win_x_end_property; > -struct drm_property *crc_win_y_end_property; > -#endif > /** > * @mst_encoders: > * > @@ -436,15 +429,6 @@ struct dm_plane_state { > struct dc_plane_state *dc_state; > }; > > -#ifdef CONFIG_DEBUG_FS > -struct crc_rec { > -uint16_t x_start; > -uint16_t y_start; > -uint16_t x_end; > -uint16_t y_end; > -}; > -#endif > - > struct dm_crtc_state { > struct drm_crtc_state base; > struct dc_stream_state *stream; > @@ -467,9 +451,6 @@ struct dm_crtc_state { > struct dc_info_packet vrr_infopacket; > > int abm_level; > -#ifdef CONFIG_DEBUG_FS > -struct crc_rec crc_window; > -#endif > }; > > #define to_dm_crtc_state(x) container_of(x, struct dm_crtc_state, base) 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 7b886a779a8c..c29dc11619f7 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 > @@ -81,41 +81,6 @@ const char *const *amdgpu_dm_crtc_get_crc_sources(struct drm_crtc *crtc, > return pipe_crc_sources; > } > > -static void amdgpu_dm_set_crc_window_default(struct dm_crtc_state *dm_crtc_state) -{ > -dm_crtc_state->crc_window.x_start = 0; > -dm_crtc_state->crc_window.y_start = 0; > -dm_crtc_state->crc_window.x_end = 0; > -dm_crtc_state->crc_window.y_end = 0; > -} > - > -bool amdgpu_dm_crc_window_is_default(struct dm_crtc_state *dm_crtc_state) -{ > -bool ret = true; > - > -if ((dm_crtc_state->crc_window.x_start != 0) || > -(dm_crtc_state->crc_window.y_start != 0) || > -(dm_crtc_state->crc_window.x_end != 0) || > -(dm_crtc_state->crc_window.y_end != 0)) > -ret = false; > - > -return ret; > -} > - > -bool amdgpu_dm_crc_window_changed(struct dm_crtc_state *dm_new_crtc_state, > -struct dm_crtc_state *dm_old_crtc_state) > -{ > -bool ret = false; > - > -if ((dm_new_crtc_state->crc_window.x_start != dm_old_crtc_state->crc_window.x_start) || > -(dm_new_crtc_state->crc_window.y_start != dm_old_crtc_state->crc_window.y_start) || > -(dm_new_crtc_state->crc_window.x_end != dm_old_crtc_state->crc_window.x_end) || > -(dm_new_crtc_state->crc_window.y_end != dm_old_crtc_state->crc_window.y_end)) > -ret = true; > - > -return ret; > -} > - > int > amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src_name, > size_t *values_cnt) > @@ -140,7 +105,6 @@ int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc, > struct dc_stream_state *stream_state = dm_crtc_state->stream; > bool enable = amdgpu_dm_is_valid_crc_source(source); > int ret = 0; > -struct crc_params *crc_window = NULL, tmp_window; > > /* Configuration will be deferred to stream enable. */ > if (!stream_state) > @@ -149,25 +113,9 @@ int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc, > mutex_lock(&adev->dm.dc_lock); > > /* Enable CRTC CRC generation if necessary. */ > -if (dm_is_crc_source_crtc(source) || source == AMDGPU_DM_PIPE_CRC_SOURCE_NONE) { > -if (!enable) > -amdgpu_dm_set_crc_window_default(dm_crtc_state); > - > -if (!amdgpu_dm_crc_window_is_default(dm_crtc_state)) { > -crc_window = &tmp_window; > - > -tmp_window.windowa_x_start = dm_crtc_state->crc_window.x_start; > -tmp_window.windowa_y_start = dm_crtc_state->crc_window.y_start; > -tmp_window.windowa_x_end = dm_crtc_state->crc_window.x_end; > -tmp_window.windowa_y_end = dm_crtc_state->crc_window.y_end; > -tmp_window.windowb_x_start = dm_crtc_state->crc_window.x_start; > -tmp_window.windowb_y_start = dm_crtc_state->crc_window.y_start; > -tmp_window.windowb_x_end = dm_crtc_state->crc_window.x_end; > -tmp_window.windowb_y_end = dm_crtc_state->crc_window.y_end; > -} > - > +if (dm_is_crc_source_crtc(source)) { Should still keep the condition "source == AMDGPU_DM_PIPE_CRC_SOURCE_NONE", this solves the problem that it doesn't really turn off crc generation. > if (!dc_stream_configure_crc(stream_state->ctx->dc, > - stream_state, crc_window, enable, enable)) { > + stream_state, NULL, enable, enable)) { > ret = -EINVAL; > goto unlock; > } > 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 0235bfb246e5..f7d731797d3f 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 > @@ -47,9 +47,6 @@ static inline bool amdgpu_dm_is_valid_crc_source(enum amdgpu_dm_pipe_crc_source > > /* amdgpu_dm_crc.c */ > #ifdef CONFIG_DEBUG_FS > -bool amdgpu_dm_crc_window_is_default(struct dm_crtc_state *dm_crtc_state); -bool amdgpu_dm_crc_window_changed(struct > dm_crtc_state *dm_new_crtc_state, > -struct dm_crtc_state *dm_old_crtc_state); > int amdgpu_dm_crtc_configure_crc_source(struct drm_crtc *crtc, > struct dm_crtc_state *dm_crtc_state, > enum amdgpu_dm_pipe_crc_source source); diff --git > a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c > index f4a2088ab179..da292cea0f56 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c > @@ -3272,6 +3272,8 @@ void core_link_enable_stream( > } > } > > +dc->hwss.enable_audio_stream(pipe_ctx); Original patch doesn't add this line. > + > /* turn off otg test pattern if enable */ > if (pipe_ctx->stream_res.tg->funcs->set_test_pattern) > pipe_ctx->stream_res.tg->funcs->set_test_pattern(pipe_ctx->stream_res.tg, > -- > 2.25.1 Thanks! Wayne Lin _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel