| Message ID | 20191002154349.26895-1-andrzej.p@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Fix comment doc for format_modifiers | expand |
On Wed, Oct 02, 2019 at 05:43:49PM +0200, Andrzej Pietrasiewicz wrote: > To human readers > > "array of struct drm_format modifiers" is almost indistinguishable from > "array of struct drm_format_modifiers", Unless I'm blind those two *are* indistinguishable :P > especially given that > struct drm_format_modifier does exist. > > And indeed the parameter passes an array of uint64_t rather than an array > of structs, but the first words of the comment suggest that it passes > an array of structs. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > --- > drivers/gpu/drm/drm_plane.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index d6ad60ab0d38..df05d8a0dd63 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane Looks like you have a broken version of git. > * @funcs: callbacks for the new plane > * @formats: array of supported formats (DRM_FORMAT\_\*) > * @format_count: number of elements in @formats > - * @format_modifiers: array of struct drm_format modifiers terminated by > + * @format_modifiers: array of modifiers of struct drm_format terminated by Now it seems to be saying it's passing in struct drm_format foo[]. That doesn't seem right either. > * DRM_FORMAT_MOD_INVALID > * @type: type of plane (overlay, primary, cursor) > * @name: printf style format string for the plane name, or NULL for default name > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Ville, W dniu 02.10.2019 o 18:18, Ville Syrjälä pisze: > On Wed, Oct 02, 2019 at 05:43:49PM +0200, Andrzej Pietrasiewicz wrote: >> To human readers >> >> "array of struct drm_format modifiers" is almost indistinguishable from >> "array of struct drm_format_modifiers", > > Unless I'm blind those two *are* indistinguishable :P > >> especially given that >> struct drm_format_modifier does exist. >> >> And indeed the parameter passes an array of uint64_t rather than an array >> of structs, but the first words of the comment suggest that it passes >> an array of structs. >> >> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> >> --- >> drivers/gpu/drm/drm_plane.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c >> index d6ad60ab0d38..df05d8a0dd63 100644 >> --- a/drivers/gpu/drm/drm_plane.c >> +++ b/drivers/gpu/drm/drm_plane.c >> @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane > > Looks like you have a broken version of git. > >> * @funcs: callbacks for the new plane >> * @formats: array of supported formats (DRM_FORMAT\_\*) >> * @format_count: number of elements in @formats >> - * @format_modifiers: array of struct drm_format modifiers terminated by >> + * @format_modifiers: array of modifiers of struct drm_format terminated by > > Now it seems to be saying it's passing in struct drm_format foo[]. > That doesn't seem right either. Good point! array of modifiers applied to struct drm_format? Andrzej
On Wed, Oct 02, 2019 at 06:22:23PM +0200, Andrzej Pietrasiewicz wrote: > Hi Ville, > > W dniu 02.10.2019 o 18:18, Ville Syrjälä pisze: > > On Wed, Oct 02, 2019 at 05:43:49PM +0200, Andrzej Pietrasiewicz wrote: > >> To human readers > >> > >> "array of struct drm_format modifiers" is almost indistinguishable from > >> "array of struct drm_format_modifiers", > > > > Unless I'm blind those two *are* indistinguishable :P > > > >> especially given that > >> struct drm_format_modifier does exist. > >> > >> And indeed the parameter passes an array of uint64_t rather than an array > >> of structs, but the first words of the comment suggest that it passes > >> an array of structs. > >> > >> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > >> --- > >> drivers/gpu/drm/drm_plane.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > >> index d6ad60ab0d38..df05d8a0dd63 100644 > >> --- a/drivers/gpu/drm/drm_plane.c > >> +++ b/drivers/gpu/drm/drm_plane.c > >> @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane > > > > Looks like you have a broken version of git. > > > >> * @funcs: callbacks for the new plane > >> * @formats: array of supported formats (DRM_FORMAT\_\*) > >> * @format_count: number of elements in @formats > >> - * @format_modifiers: array of struct drm_format modifiers terminated by > >> + * @format_modifiers: array of modifiers of struct drm_format terminated by > > > > Now it seems to be saying it's passing in struct drm_format foo[]. > > That doesn't seem right either. > > Good point! > > array of modifiers applied to struct drm_format? Not sure what this has to do with that struct? I think I'd just make it "array of modifiers terminated by ..."
Hi Andrzej Always good to have clear documentation. On Wed, Oct 02, 2019 at 05:43:49PM +0200, Andrzej Pietrasiewicz wrote: > To human readers > > "array of struct drm_format modifiers" is almost indistinguishable from > "array of struct drm_format_modifiers", especially given that > struct drm_format_modifier does exist. > > And indeed the parameter passes an array of uint64_t rather than an array > of structs, but the first words of the comment suggest that it passes > an array of structs. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > --- > drivers/gpu/drm/drm_plane.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index d6ad60ab0d38..df05d8a0dd63 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane > * @funcs: callbacks for the new plane > * @formats: array of supported formats (DRM_FORMAT\_\*) > * @format_count: number of elements in @formats > - * @format_modifiers: array of struct drm_format modifiers terminated by > + * @format_modifiers: array of modifiers of struct drm_format terminated by > * DRM_FORMAT_MOD_INVALID @format_modifiers is an array of DRM_FORMAT_* which are defined as: #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \ ((__u32)(c) << 16) | ((__u32)(d) << 24)) But the array is a u64[] like this: static const u64 tegra20_modifiers[] = { DRM_FORMAT_MOD_LINEAR, DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED, DRM_FORMAT_MOD_INVALID }; So this is not struct drm_format. Can you try to give it a shot more? Sam
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index d6ad60ab0d38..df05d8a0dd63 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -160,7 +160,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane * @funcs: callbacks for the new plane * @formats: array of supported formats (DRM_FORMAT\_\*) * @format_count: number of elements in @formats - * @format_modifiers: array of struct drm_format modifiers terminated by + * @format_modifiers: array of modifiers of struct drm_format terminated by * DRM_FORMAT_MOD_INVALID * @type: type of plane (overlay, primary, cursor) * @name: printf style format string for the plane name, or NULL for default name
To human readers "array of struct drm_format modifiers" is almost indistinguishable from "array of struct drm_format_modifiers", especially given that struct drm_format_modifier does exist. And indeed the parameter passes an array of uint64_t rather than an array of structs, but the first words of the comment suggest that it passes an array of structs. Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> --- drivers/gpu/drm/drm_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)