On Mon, 23 Oct 2023, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> On Mon, 16 Oct 2023, Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>> From: Arnd Bergmann <arnd@xxxxxxxx>
>>
>> The newly added memset() causes a warning for some reason I could not figure out:
>>
>> In file included from arch/x86/include/asm/string.h:3,
>> from drivers/gpu/drm/i915/gt/intel_rc6.c:6:
>> In function 'rc6_res_reg_init',
>> inlined from 'intel_rc6_init' at drivers/gpu/drm/i915/gt/intel_rc6.c:610:2:
>> arch/x86/include/asm/string_32.h:195:29: error: '__builtin_memset' writing 16 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
>> 195 | #define memset(s, c, count) __builtin_memset(s, c, count)
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/i915/gt/intel_rc6.c:584:9: note: in expansion of macro 'memset'
>> 584 | memset(rc6->res_reg, INVALID_MMIO_REG.reg, sizeof(rc6->res_reg));
>> | ^~~~~~
>> In function 'intel_rc6_init':
>>
>> Change it to an normal initializer and an added memcpy() that does not have
>> this problem.
>>
>> Fixes: 4bb9ca7ee0745 ("drm/i915/mtl: C6 residency and C state type for MTL SAMedia")
>> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/gt/intel_rc6.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> index 8b67abd720be8..7090e4be29cb6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>> @@ -581,19 +581,23 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
>>
>> static void rc6_res_reg_init(struct intel_rc6 *rc6)
>> {
>> - memset(rc6->res_reg, INVALID_MMIO_REG.reg, sizeof(rc6->res_reg));
>
> That's just bollocks. memset() is byte granularity, while
> INVALID_MMIO_REG.reg is u32. If the value was anything other than 0,
> this would break.
>
> And you're not supposed to look at the guts of i915_reg_t to begin with,
> that's why it's a typedef. Basically any code that accesses the members
> of i915_reg_t outside of its implementation are doing it wrong.
>
> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Thanks for the patch, pushed to drm-intel-gt-next.
BR,
Jani.
>
>
>> + i915_reg_t res_reg[INTEL_RC6_RES_MAX] = {
>> + [0 ... INTEL_RC6_RES_MAX - 1] = INVALID_MMIO_REG,
>> + };
>>
>> switch (rc6_to_gt(rc6)->type) {
>> case GT_MEDIA:
>> - rc6->res_reg[INTEL_RC6_RES_RC6] = MTL_MEDIA_MC6;
>> + res_reg[INTEL_RC6_RES_RC6] = MTL_MEDIA_MC6;
>> break;
>> default:
>> - rc6->res_reg[INTEL_RC6_RES_RC6_LOCKED] = GEN6_GT_GFX_RC6_LOCKED;
>> - rc6->res_reg[INTEL_RC6_RES_RC6] = GEN6_GT_GFX_RC6;
>> - rc6->res_reg[INTEL_RC6_RES_RC6p] = GEN6_GT_GFX_RC6p;
>> - rc6->res_reg[INTEL_RC6_RES_RC6pp] = GEN6_GT_GFX_RC6pp;
>> + res_reg[INTEL_RC6_RES_RC6_LOCKED] = GEN6_GT_GFX_RC6_LOCKED;
>> + res_reg[INTEL_RC6_RES_RC6] = GEN6_GT_GFX_RC6;
>> + res_reg[INTEL_RC6_RES_RC6p] = GEN6_GT_GFX_RC6p;
>> + res_reg[INTEL_RC6_RES_RC6pp] = GEN6_GT_GFX_RC6pp;
>> break;
>> }
>> +
>> + memcpy(rc6->res_reg, res_reg, sizeof(res_reg));
>> }
>>
>> void intel_rc6_init(struct intel_rc6 *rc6)
--
Jani Nikula, Intel
