| Message ID | CANiq72mjc5t4n25SQvYSrOEhxxpXYPZ4pPzneSJHEnc3qApu2Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable 'out' set but not used | expand |
On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > Hi, > > In today's next, I got: > > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable > 'out' set but not used [-Werror,-Wunused-but-set-variable] > > `out` seems to be there since commit 64d6255650d4 ("drm/msm: More > fully implement devcoredump for a7xx"). > > Untested diff below assuming `dumper->iova` is constant -- if you want > a formal patch, please let me know. Please send a proper patch that we can pick up. > > Cheers, > Miguel > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > index 1f5245fc2cdc..a847a0f7a73c 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, > (block->type << 8) | i); > > in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, > - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); > + block->size, out); > > out += block->size * sizeof(u32); > }
On 3/26/2024 11:19 AM, Dmitry Baryshkov wrote: > On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: >> >> Hi, >> >> In today's next, I got: >> >> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable >> 'out' set but not used [-Werror,-Wunused-but-set-variable] >> >> `out` seems to be there since commit 64d6255650d4 ("drm/msm: More >> fully implement devcoredump for a7xx"). >> >> Untested diff below assuming `dumper->iova` is constant -- if you want >> a formal patch, please let me know. > > Please send a proper patch that we can pick up. > This should be fixed with https://patchwork.freedesktop.org/patch/581853/. We can pickup that one with a Fixes tag applied. >> >> Cheers, >> Miguel >> >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> index 1f5245fc2cdc..a847a0f7a73c 100644 >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >> @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, >> (block->type << 8) | i); >> >> in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, >> - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); >> + block->size, out); >> >> out += block->size * sizeof(u32); >> } > > >
On Tue, Mar 26, 2024 at 7:31 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > This should be fixed with https://patchwork.freedesktop.org/patch/581853/. Ah, so in that case the `CRASHDUMP_READ` target should really be constant, unlike in other cases in that file? > We can pickup that one with a Fixes tag applied. Thanks! Cheers, Miguel
On Tue, 26 Mar 2024 at 20:31, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 3/26/2024 11:19 AM, Dmitry Baryshkov wrote: > > On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda > > <miguel.ojeda.sandonis@gmail.com> wrote: > >> > >> Hi, > >> > >> In today's next, I got: > >> > >> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable > >> 'out' set but not used [-Werror,-Wunused-but-set-variable] > >> > >> `out` seems to be there since commit 64d6255650d4 ("drm/msm: More > >> fully implement devcoredump for a7xx"). > >> > >> Untested diff below assuming `dumper->iova` is constant -- if you want > >> a formal patch, please let me know. > > > > Please send a proper patch that we can pick up. > > > > This should be fixed with https://patchwork.freedesktop.org/patch/581853/. Is that a correct fix? If you check other usage locations for CRASHDUMP_READ, you'll see that `out` is the last parameter and it is being incremented. > > We can pickup that one with a Fixes tag applied. > > >> > >> Cheers, > >> Miguel > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >> index 1f5245fc2cdc..a847a0f7a73c 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >> @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, > >> (block->type << 8) | i); > >> > >> in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, > >> - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); > >> + block->size, out); > >> > >> out += block->size * sizeof(u32); > >> } > > > > > >
On 3/26/2024 12:10 PM, Dmitry Baryshkov wrote: > On Tue, 26 Mar 2024 at 20:31, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 3/26/2024 11:19 AM, Dmitry Baryshkov wrote: >>> On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda >>> <miguel.ojeda.sandonis@gmail.com> wrote: >>>> >>>> Hi, >>>> >>>> In today's next, I got: >>>> >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable >>>> 'out' set but not used [-Werror,-Wunused-but-set-variable] >>>> >>>> `out` seems to be there since commit 64d6255650d4 ("drm/msm: More >>>> fully implement devcoredump for a7xx"). >>>> >>>> Untested diff below assuming `dumper->iova` is constant -- if you want >>>> a formal patch, please let me know. >>> >>> Please send a proper patch that we can pick up. >>> >> >> This should be fixed with https://patchwork.freedesktop.org/patch/581853/. > > Is that a correct fix? If you check other usage locations for > CRASHDUMP_READ, you'll see that `out` is the last parameter and it is > being incremented. > Right but in this function out is not the last parameter of CRASHDUMP_READ. Maybe you or Rob can correct me but I thought the fix looked sane although noone commented on that patch. >> >> We can pickup that one with a Fixes tag applied. >> >>>> >>>> Cheers, >>>> Miguel >>>> >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>> index 1f5245fc2cdc..a847a0f7a73c 100644 >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>> @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, >>>> (block->type << 8) | i); >>>> >>>> in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, >>>> - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); >>>> + block->size, out); >>>> >>>> out += block->size * sizeof(u32); >>>> } >>> >>> >>> > > >
On Tue, 26 Mar 2024 at 21:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 3/26/2024 12:10 PM, Dmitry Baryshkov wrote: > > On Tue, 26 Mar 2024 at 20:31, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > >> > >> > >> > >> On 3/26/2024 11:19 AM, Dmitry Baryshkov wrote: > >>> On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda > >>> <miguel.ojeda.sandonis@gmail.com> wrote: > >>>> > >>>> Hi, > >>>> > >>>> In today's next, I got: > >>>> > >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable > >>>> 'out' set but not used [-Werror,-Wunused-but-set-variable] > >>>> > >>>> `out` seems to be there since commit 64d6255650d4 ("drm/msm: More > >>>> fully implement devcoredump for a7xx"). > >>>> > >>>> Untested diff below assuming `dumper->iova` is constant -- if you want > >>>> a formal patch, please let me know. > >>> > >>> Please send a proper patch that we can pick up. > >>> > >> > >> This should be fixed with https://patchwork.freedesktop.org/patch/581853/. > > > > Is that a correct fix? If you check other usage locations for > > CRASHDUMP_READ, you'll see that `out` is the last parameter and it is > > being incremented. > > > > Right but in this function out is not the last parameter of CRASHDUMP_READ. Yes. I think in this case the patch from this email is more correct. > > Maybe you or Rob can correct me but I thought the fix looked sane > although noone commented on that patch. > > >> > >> We can pickup that one with a Fixes tag applied. > >> > >>>> > >>>> Cheers, > >>>> Miguel > >>>> > >>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >>>> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >>>> index 1f5245fc2cdc..a847a0f7a73c 100644 > >>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > >>>> @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, > >>>> (block->type << 8) | i); > >>>> > >>>> in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, > >>>> - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); > >>>> + block->size, out); > >>>> > >>>> out += block->size * sizeof(u32); > >>>> } > >>> > >>> > >>> > > > > > >
On Tue, Mar 26, 2024 at 7:47 PM Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Tue, 26 Mar 2024 at 21:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > > > > > On 3/26/2024 12:10 PM, Dmitry Baryshkov wrote: > > > On Tue, 26 Mar 2024 at 20:31, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > >> > > >> > > >> > > >> On 3/26/2024 11:19 AM, Dmitry Baryshkov wrote: > > >>> On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda > > >>> <miguel.ojeda.sandonis@gmail.com> wrote: > > >>>> > > >>>> Hi, > > >>>> > > >>>> In today's next, I got: > > >>>> > > >>>> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable > > >>>> 'out' set but not used [-Werror,-Wunused-but-set-variable] > > >>>> > > >>>> `out` seems to be there since commit 64d6255650d4 ("drm/msm: More > > >>>> fully implement devcoredump for a7xx"). > > >>>> > > >>>> Untested diff below assuming `dumper->iova` is constant -- if you want > > >>>> a formal patch, please let me know. > > >>> > > >>> Please send a proper patch that we can pick up. > > >>> > > >> > > >> This should be fixed with https://patchwork.freedesktop.org/patch/581853/. > > > > > > Is that a correct fix? If you check other usage locations for > > > CRASHDUMP_READ, you'll see that `out` is the last parameter and it is > > > being incremented. > > > > > > > Right but in this function out is not the last parameter of CRASHDUMP_READ. > > Yes. I think in this case the patch from this email is more correct. Yes, this patch is more correct than the other one. I tried to fix a bug with a6xx that I noticed while adding support for a7xx, which I forgot to split out from "drm/msm: More fully implement devcoredump for a7xx" into a separate commit, and this hunk was missing. Sorry about that. Connor
On 3/26/2024 12:47 PM, Dmitry Baryshkov wrote: > On Tue, 26 Mar 2024 at 21:32, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >> >> >> >> On 3/26/2024 12:10 PM, Dmitry Baryshkov wrote: >>> On Tue, 26 Mar 2024 at 20:31, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: >>>> >>>> >>>> >>>> On 3/26/2024 11:19 AM, Dmitry Baryshkov wrote: >>>>> On Tue, 26 Mar 2024 at 20:05, Miguel Ojeda >>>>> <miguel.ojeda.sandonis@gmail.com> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> In today's next, I got: >>>>>> >>>>>> drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable >>>>>> 'out' set but not used [-Werror,-Wunused-but-set-variable] >>>>>> >>>>>> `out` seems to be there since commit 64d6255650d4 ("drm/msm: More >>>>>> fully implement devcoredump for a7xx"). >>>>>> >>>>>> Untested diff below assuming `dumper->iova` is constant -- if you want >>>>>> a formal patch, please let me know. >>>>> >>>>> Please send a proper patch that we can pick up. >>>>> >>>> >>>> This should be fixed with https://patchwork.freedesktop.org/patch/581853/. >>> >>> Is that a correct fix? If you check other usage locations for >>> CRASHDUMP_READ, you'll see that `out` is the last parameter and it is >>> being incremented. >>> >> >> Right but in this function out is not the last parameter of CRASHDUMP_READ. > > Yes. I think in this case the patch from this email is more correct. > Alright, in that case, Miguel can you please repost this with the Fixes tags and in a patch form. >> >> Maybe you or Rob can correct me but I thought the fix looked sane >> although noone commented on that patch. > >> >>>> >>>> We can pickup that one with a Fixes tag applied. >>>> >>>>>> >>>>>> Cheers, >>>>>> Miguel >>>>>> >>>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>>>> b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>>>> index 1f5245fc2cdc..a847a0f7a73c 100644 >>>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c >>>>>> @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, >>>>>> (block->type << 8) | i); >>>>>> >>>>>> in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, >>>>>> - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); >>>>>> + block->size, out); >>>>>> >>>>>> out += block->size * sizeof(u32); >>>>>> } >>>>> >>>>> >>>>> >>> >>> >>> > > >
On Tue, Mar 26, 2024 at 8:56 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Alright, in that case, Miguel can you please repost this with the Fixes > tags and in a patch form. Done at https://lore.kernel.org/lkml/20240326212324.185832-1-ojeda@kernel.org/ Thanks all! Cheers, Miguel
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c index 1f5245fc2cdc..a847a0f7a73c 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c @@ -852,7 +852,7 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu, (block->type << 8) | i); in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE, - block->size, dumper->iova + A6XX_CD_DATA_OFFSET); + block->size, out); out += block->size * sizeof(u32); }
Hi, In today's next, I got: drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable 'out' set but not used [-Werror,-Wunused-but-set-variable] `out` seems to be there since commit 64d6255650d4 ("drm/msm: More fully implement devcoredump for a7xx"). Untested diff below assuming `dumper->iova` is constant -- if you want a formal patch, please let me know. Cheers, Miguel