Hi Linus,
Just some small i915 and amdgpu fixes this week, Should be all until you open the merge window.
Regards, Dave.
drm-fixes-2021-04-23: drm fixes for 5.12 final
amdgpu: - Fix gpuvm page table update issue - Modifier fixes - Register fix for dimgrey cavefish
i915: - GVT's BDW regression fix for cmd parser - Fix modesetting in case of unexpected AUX timeouts The following changes since commit bf05bf16c76bb44ab5156223e1e58e26dfe30a88:
Linux 5.12-rc8 (2021-04-18 14:45:32 -0700)
are available in the Git repository at:
git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-04-23
for you to fetch changes up to aca38735ae624b93c71c055b68d5802b8f356ea5:
Merge tag 'drm-intel-fixes-2021-04-22' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2021-04-23 12:18:21 +1000)
---------------------------------------------------------------- drm fixes for 5.12 final
amdgpu: - Fix gpuvm page table update issue - Modifier fixes - Register fix for dimgrey cavefish
i915: - GVT's BDW regression fix for cmd parser - Fix modesetting in case of unexpected AUX timeouts
---------------------------------------------------------------- Dave Airlie (2): Merge tag 'amd-drm-fixes-5.12-2021-04-21' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Merge tag 'drm-intel-fixes-2021-04-22' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes
Imre Deak (1): drm/i915: Fix modesetting in case of unexpected AUX timeouts
Jiansong Chen (1): drm/amdgpu: fix GCR_GENERAL_CNTL offset for dimgrey_cavefish
Philip Yang (1): drm/amdgpu: reserve fence slot to update page table
Qingqing Zhuo (1): drm/amd/display: Update modifier list for gfx10_3
Rodrigo Vivi (1): Merge tag 'gvt-fixes-2021-04-20' of https://github.com/intel/gvt-linux into drm-intel-fixes
Simon Ser (1): amd/display: allow non-linear multi-planar formats
Zhenyu Wang (1): drm/i915/gvt: Fix BDW command parser regression
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++++++++-- drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 2 +- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++++++--------- drivers/gpu/drm/i915/display/intel_dp_link_training.c | 3 ++- drivers/gpu/drm/i915/gvt/cmd_parser.c | 19 +++++++++++++------ 5 files changed, 30 insertions(+), 19 deletions(-)
The pull request you sent on Fri, 23 Apr 2021 13:52:29 +1000:
git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2021-04-23
has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/5bfc75d92efd494db37f5c4c173d3639d4772966
Thank you!
I've updated to Fedora 34 on one of my machines, and it causes a lot of i915 warnings like
drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’: drivers/gpu/drm/i915/intel_pm.c:3059:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’} drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
and the reason is that gcc now seems to look at the argument array size more, and notices that
(a) intel_print_wm_latency() takes a "const u16 wm[8]" argument
but
(b) most of the arrays passed in tend to look like 'u16 pri_latency[5]'
I think I will make the argument type to intel_print_wm_latency() be just "const u16 wm[]" for now, just to avoid seeing a ton of silly warnings.
I'm not sure if there is a better solution (like making all of those latency arrays be 8 entries in size), so I'm just letting you know about my change in this area in case anybody has a better idea.
Linus
On Tue, Apr 27, 2021 at 4:43 PM Linus Torvalds torvalds@linux-foundation.org wrote:
I think I will make the argument type to intel_print_wm_latency() be just "const u16 wm[]" for now, just to avoid seeing a ton of silly warnings.
After fixing the trivial ones, this one remains:
drivers/gpu/drm/i915/display/intel_dp.c: In function ‘intel_dp_check_mst_status’: drivers/gpu/drm/i915/display/intel_dp.c:4554:22: warning: ‘drm_dp_channel_eq_ok’ reading 6 bytes from a region of size 4 [-Wstringop-overread] 4554 | !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_dp.c:4554:22: note: referencing argument 1 of type ‘const u8 *’ {aka ‘const unsigned char *’} In file included from drivers/gpu/drm/i915/display/intel_dp.c:38: ./include/drm/drm_dp_helper.h:1459:6: note: in a call to function ‘drm_dp_channel_eq_ok’ 1459 | bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE], | ^~~~~~~~~~~~~~~~~~~~
and I'm not fixing that one, because it actually looks like a valid warning, and doesn't have an obvious fix.
That "esi[]" array is 14 bytes in size (DP_DPRX_ESI_LEN). So when it does that "&esi[10]" and passes it in as an argument, then only 4 bytes remain of the array.
And drm_dp_channel_eq_ok() supposedly takes a "const u8 link_status[DP_LINK_STATUS_SIZE]", which is 6 bytes.
There may be some reason this is ok, but it does look a bit fishy, and the compiler warning is appropriate.
Linus
On Tue, 27 Apr 2021, Linus Torvalds torvalds@linux-foundation.org wrote:
I've updated to Fedora 34 on one of my machines, and it causes a lot of i915 warnings like
drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’: drivers/gpu/drm/i915/intel_pm.c:3059:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’} drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
and the reason is that gcc now seems to look at the argument array size more, and notices that
Arnd Bergmann reported some of these a while back. I think we have some of them fixed in our -next already, but not all. Thanks for the reminder.
BR, Jani.
I have heard nothing about this, and it remains the only warning from my allmodconfig build (I have another one for drm compiled with clang, but there I at least heard back that a fix exists).
Since I am going to release rc1 tomorrow, and I don't want to release it with an ugly compiler warning, I took it upon myself to just fix the code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
HOWEVER.
That commit fixes the warning, and is at worst harmless. At best it fixes an access to random stack memory. But it does smell like somebody who actually knows how these arrays work should look at that code.
IOW, maybe the code should actually have read 16 bytes from the Event Status Indicator? Maybe offset 10 was wrong? Maybe drm_dp_channel_eq_ok() should never have taken six bytes to begin with?
It's a mystery, and I haven't heard anything otherwise, so there it is.
Linus
On Wed, Apr 28, 2021 at 12:27 AM Jani Nikula jani.nikula@intel.com wrote:
On Tue, 27 Apr 2021, Linus Torvalds torvalds@linux-foundation.org wrote:
I've updated to Fedora 34 on one of my machines, and it causes a lot of i915 warnings like
drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’: drivers/gpu/drm/i915/intel_pm.c:3059:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’} drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
and the reason is that gcc now seems to look at the argument array size more, and notices that
Arnd Bergmann reported some of these a while back. I think we have some of them fixed in our -next already, but not all. Thanks for the reminder.
On Sat, 08 May 2021, Linus Torvalds torvalds@linux-foundation.org wrote:
I have heard nothing about this, and it remains the only warning from my allmodconfig build (I have another one for drm compiled with clang, but there I at least heard back that a fix exists).
Since I am going to release rc1 tomorrow, and I don't want to release it with an ugly compiler warning, I took it upon myself to just fix the code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
HOWEVER.
That commit fixes the warning, and is at worst harmless. At best it fixes an access to random stack memory. But it does smell like somebody who actually knows how these arrays work should look at that code.
IOW, maybe the code should actually have read 16 bytes from the Event Status Indicator? Maybe offset 10 was wrong? Maybe drm_dp_channel_eq_ok() should never have taken six bytes to begin with?
It's a mystery, and I haven't heard anything otherwise, so there it is.
Fair enough. My bad for not getting this fixed.
The fix is harmless. drm_dp_channel_eq_ok() only ever accesses 3 bytes instead of 6. I figure the DP_LINK_STATUS_SIZE (=6) is there because in the normal case you'd read that much, and use a family of functions on that data, some of which do access the full 6 bytes, some don't.
In our case, we use drm_dp_channel_eq_ok() to check 3 bytes of similarly encoded data elsewhere in the DPCD address space, and the DP_LINK_STATUS_SIZE is meaningless there.
The straightforward fix would be to replace link_status[DP_LINK_STATUS_SIZE] with link_status[3], and that likely needs changes in dp_link_status() and dp_get_lane_status() as well.
BR, Jani.
LinusOn Wed, Apr 28, 2021 at 12:27 AM Jani Nikula jani.nikula@intel.com wrote:
On Tue, 27 Apr 2021, Linus Torvalds torvalds@linux-foundation.org wrote:
I've updated to Fedora 34 on one of my machines, and it causes a lot of i915 warnings like
drivers/gpu/drm/i915/intel_pm.c: In function ‘ilk_setup_wm_latency’: drivers/gpu/drm/i915/intel_pm.c:3059:9: note: referencing argument 3 of type ‘const u16 *’ {aka ‘const short unsigned int *’} drivers/gpu/drm/i915/intel_pm.c:2994:13: note: in a call to function ‘intel_print_wm_latency’
and the reason is that gcc now seems to look at the argument array size more, and notices that
Arnd Bergmann reported some of these a while back. I think we have some of them fixed in our -next already, but not all. Thanks for the reminder.
dri-devel@lists.freedesktop.org
-
Dave Airlie -
Jani Nikula -
Linus Torvalds -
pr-tracker-bot＠kernel.org