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.
> > }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)))
| ^~~~~~
