[kbuild-all] Re: [PATCH] drm: Update MST First Link Slot Information Based on Encoding Format

Hi Bhawanpreet, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip linus/master v5.15-rc5 next-20211013] [cannot apply to drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Bhawanpreet-Lakha/drm-Update-MST... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-a003-20211012 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/5604bf980dcbfdd7650b7e1d5d4a2fd9f... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Bhawanpreet-Lakha/drm-Update-MST-First-Link-Slot-Information-Based-on-Encoding-Format/20211013-060001 git checkout 5604bf980dcbfdd7650b7e1d5d4a2fd9f18cd866 # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp(a)intel.com&gt; All errors (new ones prefixed by >>): In file included from drivers/gpu/drm/amd/amdgpu/../display/dmub/dmub_srv.h:67, from drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:35: drivers/gpu/drm/amd/amdgpu/../display/dmub/inc/dmub_cmd.h: In function 'dmub_rb_flush_pending': drivers/gpu/drm/amd/amdgpu/../display/dmub/inc/dmub_cmd.h:2819:12: warning: variable 'temp' set but not used [-Wunused-but-set-variable] 2819 | uint64_t temp; | ^~~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: At top level: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:633:6: warning: no previous prototype for 'dmub_aux_setconfig_callback' [-Wmissing-prototypes] 633 | void dmub_aux_setconfig_callback(struct amdgpu_device *adev, struct dmub_notification *notify) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:649:6: warning: no previous prototype for 'dmub_hpd_callback' [-Wmissing-prototypes] 649 | void dmub_hpd_callback(struct amdgpu_device *adev, struct dmub_notification *notify) | ^~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:706:6: warning: no previous prototype for 'register_dmub_notify_callback' [-Wmissing-prototypes] 706 | bool register_dmub_notify_callback(struct amdgpu_device *adev, enum dmub_notification_type type, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'dm_update_mst_vcpi_slots_for_dsc': drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7174:12: warning: variable 'clock' set but not used [-Wunused-but-set-variable] 7174 | int i, j, clock; | ^~~~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function 'amdgpu_dm_atomic_check': 10912 | link_coding_cap = dc_link_dp_mst_decide_link_encoding_format(aconnector->dc_link); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: At top level: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:11394:5: warning: no previous prototype for 'amdgpu_dm_set_dmub_async_sync_status' [-Wmissing-prototypes] 11394 | int amdgpu_dm_set_dmub_async_sync_status(bool is_cmd_aux, struct dc_context *ctx, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +/dc_link_dp_mst_decide_link_encoding_format +10912 drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c 10643 10644 /** 10645 * amdgpu_dm_atomic_check() - Atomic check implementation for AMDgpu DM. 10646 * @dev: The DRM device 10647 * @state: The atomic state to commit 10648 * 10649 * Validate that the given atomic state is programmable by DC into hardware. 10650 * This involves constructing a &struct dc_state reflecting the new hardware 10651 * state we wish to commit, then querying DC to see if it is programmable. It's 10652 * important not to modify the existing DC state. Otherwise, atomic_check 10653 * may unexpectedly commit hardware changes. 10654 * 10655 * When validating the DC state, it's important that the right locks are 10656 * acquired. For full updates case which removes/adds/updates streams on one 10657 * CRTC while flipping on another CRTC, acquiring global lock will guarantee 10658 * that any such full update commit will wait for completion of any outstanding 10659 * flip using DRMs synchronization events. 10660 * 10661 * Note that DM adds the affected connectors for all CRTCs in state, when that 10662 * might not seem necessary. This is because DC stream creation requires the 10663 * DC sink, which is tied to the DRM connector state. Cleaning this up should 10664 * be possible but non-trivial - a possible TODO item. 10665 * 10666 * Return: -Error code if validation failed. 10667 */ 10668 static int amdgpu_dm_atomic_check(struct drm_device *dev, 10669 struct drm_atomic_state *state) 10670 { 10671 struct amdgpu_device *adev = drm_to_adev(dev); 10672 struct dm_atomic_state *dm_state = NULL; 10673 struct dc *dc = adev->dm.dc; 10674 struct drm_connector *connector; 10675 struct drm_connector_state *old_con_state, *new_con_state; 10676 struct drm_crtc *crtc; 10677 struct drm_crtc_state *old_crtc_state, *new_crtc_state; 10678 struct drm_plane *plane; 10679 struct drm_plane_state *old_plane_state, *new_plane_state; 10680 enum dc_status status; 10681 int ret, i; 10682 bool lock_and_validation_needed = false; 10683 struct dm_crtc_state *dm_old_crtc_state; 10684 #if defined(CONFIG_DRM_AMD_DC_DCN) 10685 struct dsc_mst_fairness_vars vars[MAX_PIPES]; 10686 #endif 10687 struct drm_dp_mst_topology_state *mst_state; 10688 struct drm_dp_mst_topology_mgr *mgr; 10689 10690 trace_amdgpu_dm_atomic_check_begin(state); 10691 10692 ret = drm_atomic_helper_check_modeset(dev, state); 10693 if (ret) 10694 goto fail; 10695 10696 /* Check connector changes */ 10697 for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { 10698 struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state); 10699 struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); 10700 10701 /* Skip connectors that are disabled or part of modeset already. */ 10702 if (!old_con_state->crtc && !new_con_state->crtc) 10703 continue; 10704 10705 if (!new_con_state->crtc) 10706 continue; 10707 10708 new_crtc_state = drm_atomic_get_crtc_state(state, new_con_state->crtc); 10709 if (IS_ERR(new_crtc_state)) { 10710 ret = PTR_ERR(new_crtc_state); 10711 goto fail; 10712 } 10713 10714 if (dm_old_con_state->abm_level != 10715 dm_new_con_state->abm_level) 10716 new_crtc_state->connectors_changed = true; 10717 } 10718 10719 #if defined(CONFIG_DRM_AMD_DC_DCN) 10720 if (dc_resource_is_dsc_encoding_supported(dc)) { 10721 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 10722 if (drm_atomic_crtc_needs_modeset(new_crtc_state)) { 10723 ret = add_affected_mst_dsc_crtcs(state, crtc); 10724 if (ret) 10725 goto fail; 10726 } 10727 } 10728 } 10729 #endif 10730 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 10731 dm_old_crtc_state = to_dm_crtc_state(old_crtc_state); 10732 10733 if (!drm_atomic_crtc_needs_modeset(new_crtc_state) && 10734 !new_crtc_state->color_mgmt_changed && 10735 old_crtc_state->vrr_enabled == new_crtc_state->vrr_enabled && 10736 dm_old_crtc_state->dsc_force_changed == false) 10737 continue; 10738 10739 ret = amdgpu_dm_verify_lut_sizes(new_crtc_state); 10740 if (ret) 10741 goto fail; 10742 10743 if (!new_crtc_state->enable) 10744 continue; 10745 10746 ret = drm_atomic_add_affected_connectors(state, crtc); 10747 if (ret) 10748 return ret; 10749 10750 ret = drm_atomic_add_affected_planes(state, crtc); 10751 if (ret) 10752 goto fail; 10753 10754 if (dm_old_crtc_state->dsc_force_changed) 10755 new_crtc_state->mode_changed = true; 10756 } 10757 10758 /* 10759 * Add all primary and overlay planes on the CRTC to the state 10760 * whenever a plane is enabled to maintain correct z-ordering 10761 * and to enable fast surface updates. 10762 */ 10763 drm_for_each_crtc(crtc, dev) { 10764 bool modified = false; 10765 10766 for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) { 10767 if (plane->type == DRM_PLANE_TYPE_CURSOR) 10768 continue; 10769 10770 if (new_plane_state->crtc == crtc || 10771 old_plane_state->crtc == crtc) { 10772 modified = true; 10773 break; 10774 } 10775 } 10776 10777 if (!modified) 10778 continue; 10779 10780 drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) { 10781 if (plane->type == DRM_PLANE_TYPE_CURSOR) 10782 continue; 10783 10784 new_plane_state = 10785 drm_atomic_get_plane_state(state, plane); 10786 10787 if (IS_ERR(new_plane_state)) { 10788 ret = PTR_ERR(new_plane_state); 10789 goto fail; 10790 } 10791 } 10792 } 10793 10794 /* Remove exiting planes if they are modified */ 10795 for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) { 10796 ret = dm_update_plane_state(dc, state, plane, 10797 old_plane_state, 10798 new_plane_state, 10799 false, 10800 &lock_and_validation_needed); 10801 if (ret) 10802 goto fail; 10803 } 10804 10805 /* Disable all crtcs which require disable */ 10806 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 10807 ret = dm_update_crtc_state(&adev->dm, state, crtc, 10808 old_crtc_state, 10809 new_crtc_state, 10810 false, 10811 &lock_and_validation_needed); 10812 if (ret) 10813 goto fail; 10814 } 10815 10816 /* Enable all crtcs which require enable */ 10817 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 10818 ret = dm_update_crtc_state(&adev->dm, state, crtc, 10819 old_crtc_state, 10820 new_crtc_state, 10821 true, 10822 &lock_and_validation_needed); 10823 if (ret) 10824 goto fail; 10825 } 10826 10827 ret = validate_overlay(state); 10828 if (ret) 10829 goto fail; 10830 10831 /* Add new/modified planes */ 10832 for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state, new_plane_state, i) { 10833 ret = dm_update_plane_state(dc, state, plane, 10834 old_plane_state, 10835 new_plane_state, 10836 true, 10837 &lock_and_validation_needed); 10838 if (ret) 10839 goto fail; 10840 } 10841 10842 /* Run this here since we want to validate the streams we created */ 10843 ret = drm_atomic_helper_check_planes(dev, state); 10844 if (ret) 10845 goto fail; 10846 10847 /* Check cursor planes scaling */ 10848 for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { 10849 ret = dm_check_crtc_cursor(state, crtc, new_crtc_state); 10850 if (ret) 10851 goto fail; 10852 } 10853 10854 if (state->legacy_cursor_update) { 10855 /* 10856 * This is a fast cursor update coming from the plane update 10857 * helper, check if it can be done asynchronously for better 10858 * performance. 10859 */ 10860 state->async_update = 10861 !drm_atomic_helper_async_check(dev, state); 10862 10863 /* 10864 * Skip the remaining global validation if this is an async 10865 * update. Cursor updates can be done without affecting 10866 * state or bandwidth calcs and this avoids the performance 10867 * penalty of locking the private state object and 10868 * allocating a new dc_state. 10869 */ 10870 if (state->async_update) 10871 return 0; 10872 } 10873 10874 /* Check scaling and underscan changes*/ 10875 /* TODO Removed scaling changes validation due to inability to commit 10876 * new stream into context w\o causing full reset. Need to 10877 * decide how to handle. 10878 */ 10879 for_each_oldnew_connector_in_state(state, connector, old_con_state, new_con_state, i) { 10880 struct dm_connector_state *dm_old_con_state = to_dm_connector_state(old_con_state); 10881 struct dm_connector_state *dm_new_con_state = to_dm_connector_state(new_con_state); 10882 struct amdgpu_crtc *acrtc = to_amdgpu_crtc(dm_new_con_state->base.crtc); 10883 10884 /* Skip any modesets/resets */ 10885 if (!acrtc || drm_atomic_crtc_needs_modeset( 10886 drm_atomic_get_new_crtc_state(state, &acrtc->base))) 10887 continue; 10888 10889 /* Skip any thing not scale or underscan changes */ 10890 if (!is_scaling_state_different(dm_new_con_state, dm_old_con_state)) 10891 continue; 10892 10893 lock_and_validation_needed = true; 10894 } 10895 10896 #if defined(CONFIG_DRM_AMD_DC_DCN) 10897 for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { 10898 struct amdgpu_dm_connector *aconnector; 10899 struct drm_connector *connector; 10900 struct drm_connector_list_iter iter; 10901 u8 link_coding_cap; 10902 10903 if (!mgr->mst_state ) 10904 continue; 10905 10906 drm_connector_list_iter_begin(dev, &iter); 10907 drm_for_each_connector_iter(connector, &iter) { 10908 int id = connector->index; 10909 10910 if (id == mst_state->mgr->conn_base_id) { 10911 aconnector = to_amdgpu_dm_connector(connector); 10912 link_coding_cap = dc_link_dp_mst_decide_link_encoding_format(aconnector->dc_link); 10913 drm_dp_mst_update_coding_cap(mst_state, link_coding_cap); 10914 10915 break; 10916 } 10917 } 10918 drm_connector_list_iter_end(&iter); 10919 10920 } 10921 #endif 10922 /** 10923 * Streams and planes are reset when there are changes that affect 10924 * bandwidth. Anything that affects bandwidth needs to go through 10925 * DC global validation to ensure that the configuration can be applied 10926 * to hardware. 10927 * 10928 * We have to currently stall out here in atomic_check for outstanding 10929 * commits to finish in this case because our IRQ handlers reference 10930 * DRM state directly - we can end up disabling interrupts too early 10931 * if we don't. 10932 * 10933 * TODO: Remove this stall and drop DM state private objects. 10934 */ 10935 if (lock_and_validation_needed) { 10936 ret = dm_atomic_get_state(state, &dm_state); 10937 if (ret) 10938 goto fail; 10939 10940 ret = do_aquire_global_lock(dev, state); 10941 if (ret) 10942 goto fail; 10943 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org