On Thu, May 30, 2024 at 6:31 AM Tasos Sahanidis <tasos@xxxxxxxxxxxx> wrote:
>
> On Mon, May 27, 2024 at 10:47:39AM -0400, Alex Deucher wrote:
> > On Mon, May 27, 2024 at 5:42 AM Tasos Sahanidis <tasos@xxxxxxxxxxxx> wrote:
> > >
> > > On 2024-05-23 17:52, Alex Deucher wrote:
> > > > On Thu, May 23, 2024 at 9:05 AM Tasos Sahanidis <tasos@xxxxxxxxxxxx> wrote:
> > > >>
> > > >> Dyanmically sized arrays used [1] instead of []. Replacing the former
> > > >> with the latter resolves multiple warnings observed on boot with a
> > > >> BONAIRE card.
> > > >>
> > > >> Signed-off-by: Tasos Sahanidis <tasos@xxxxxxxxxxxx>
> > > >> ---
> > > >> drivers/gpu/drm/amd/include/pptable.h | 24 ++++++++++++------------
> > > >> 1 file changed, 12 insertions(+), 12 deletions(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/amd/include/pptable.h b/drivers/gpu/drm/amd/include/pptable.h
> > > >> index 2e8e6c9875f6..d1dec880d2d6 100644
> > > >> --- a/drivers/gpu/drm/amd/include/pptable.h
> > > >> +++ b/drivers/gpu/drm/amd/include/pptable.h
> > > >> @@ -480,7 +480,7 @@ typedef struct _StateArray{
> > > >> //how many states we have
> > > >> UCHAR ucNumEntries;
> > > >>
> > > >> - ATOM_PPLIB_STATE_V2 states[1];
> > > >> + ATOM_PPLIB_STATE_V2 states[];
> > > >
> > > > Can you add __counted_by(ucNumEntries) to the end of the line? E.g.,
> > > >
> > > > ATOM_PPLIB_STATE_V2 states[] __counted_by(ucNumEntries);
> > > >
> > > > Same comment for the other changes below.
> > > >
> > > > Alex
> > >
>
> Apologies for the delay. I realised my compiler actually didn't support
> the attribute, so I had to install clang 19 from git.
>
> After doing so, I saw this warning about ATOM_PPLIB_STATE_V2.
>
> drivers/gpu/drm/amd/amdgpu/../include/pptable.h:483:5: warning: 'counted_by' should not be applied to an array with element of unknown size because 'ATOM_PPLIB_STATE_V2'
> (aka 'struct _ATOM_PPLIB_STATE_V2') is a struct type with a flexible array member. This will be an error in a future compiler version [-Wbounds-safety-counted-by-elt-type-unknown-size]
> 483 | ATOM_PPLIB_STATE_V2 states[] __counted_by(ucNumEntries);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Thus I'll remove the __counted_by() from that one.
just leave it there commented out. E.g.,
ATOM_PPLIB_STATE_V2 states[] /* __counted_by(ucNumEntries) */;
for reference so if the compiler ever gets updated to handle this, it
can be re-enabled.
>
> > > }NonClockInfoArray;
> > >
> > > typedef struct _ATOM_PPLIB_Clock_Voltage_Dependency_Record
> > > {
> > > USHORT usClockLow;
> > > UCHAR ucClockHigh;
> > > USHORT usVoltage;
> > >
> > > All the other ones are UCHAR, so __counted_by can not be used on them.
> >
> > I'm not following. Why not?
> >
> > Alex
>
> If I'm not mistaken, for a UCHAR flexible array such as clockInfo[],
> we would then need to multiply the count by the size, which results in:
>
> drivers/gpu/drm/amd/amdgpu/../include/pptable.h:494:36: error: 'counted_by' argument must be a simple declaration reference
> 494 | UCHAR clockInfo[] __counted_by(ucEntrySize * ucNumEntries);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/compiler_attributes.h:105:62: note: expanded from macro '__counted_by'
> 105 | # define __counted_by(member) __attribute__((__counted_by__(member)))
> | ^~~~~~
Ah, yes, we can skip those.
Alex
