Re: [PATCH 2/4] drm: Create a format/modifier blob

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17-07-14 22:10:15, Ville Syrjälä wrote:
On Fri, Jul 14, 2017 at 11:41:49AM -0700, Ben Widawsky wrote:
On 17-06-29 22:49:44, Ville Syrjälä wrote:

[snip]

>
>... but here it's ALIGN(formats_offset+formats_size). I think we should
>be aligning the same thing in both cases, or we add a BUILD_BUG_ON to
>make sure the header size always stays a multiple of 8 bytes.
>
>That's mainly because the design of the structure seems geared towards
>expanding the header in the future (as in why else would you have the
>offsets?).
>

I guess I don't quite understand what you're asking for. The first thing is
determining a size, the second is finding an offset into the blob.

If I remember my thinking correctly, then what I was trying to say was

ALIGN(multiple_of_4, 8) + ALIGN(multiple_of_4, 8) != ALIGN(multiple_of_4 + multiple_of_4, 8)

but the code was assuming that it's true, or that at least one of those
things is a multiple of 8 already.


Going with this and calling it sufficient.

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 0da6707a8cf7..5c14beee52ff 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -96,6 +96,7 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
       /* Modifiers offset is a pointer to a struct with a 64 bit field so it
        * should be naturally aligned to 8B.
        */
+       BUILD_BUG_ON(sizeof(struct drm_format_modifier_blob) % 8);
       blob_size += ALIGN(formats_size, 8);
       blob_size += modifiers_size;



I don't mind changing this, but tell me what you want.

BUILD_BUG_ON sounds good to me regardless.

[snip]

--
Ben Widawsky, Intel Open Source Technology Center

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux