On Wed, Aug 5, 2015 at 10:29 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Wed, Aug 05, 2015 at 10:17:55AM +0100, Chris Wilson wrote:
>> On Wed, Aug 05, 2015 at 11:25:43AM +0530, sourab.gupta@xxxxxxxxx wrote:
>> > @@ -555,10 +558,12 @@ static void forward_one_gen_pmu_sample(struct drm_i915_private *dev_priv,
>> > struct drm_i915_ts_node_ctx_id *ctx_info;
>> > struct drm_i915_ts_node_ring_id *ring_info;
>> > struct drm_i915_ts_node_pid *pid_info;
>> > + struct drm_i915_ts_node_tag *tag_info;
>> > struct perf_raw_record raw;
>> >
>> > BUILD_BUG_ON((TS_DATA_SIZE != 8) || (CTX_INFO_SIZE != 8) ||
>> > - (RING_INFO_SIZE != 8) || (PID_INFO_SIZE != 8));
>> > + (RING_INFO_SIZE != 8) || (PID_INFO_SIZE != 8) ||
>> > + (TAG_INFO_SIZE != 8));
>>
>> This is much more useful if each clause is independent. The error
>> message is then unambiguous and it looks neater.
>>
>> > snapshot = dev_priv->gen_pmu.buffer.addr + node->offset;
>> > snapshot_size = TS_DATA_SIZE + CTX_INFO_SIZE;
>>
>> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>> > index 3dcc862..db91098 100644
>> > --- a/include/uapi/drm/i915_drm.h
>> > +++ b/include/uapi/drm/i915_drm.h
>> > @@ -104,7 +104,8 @@ struct drm_i915_gen_pmu_attr {
>> > __u32 size;
>> > __u32 sample_ring:1,
>> > sample_pid:1,
>> > - __reserved_1:30;
>> > + sample_tag:1,
>> > + __reserved_1:29;
>>
>> Start each bitfield entry on its own line with __u32;
>
> also no bitfields in uapi headers.
> -Daniel
Ah, I had previously asked Sourab to pack the bitfields into the same
u64. I think we only get into undefined ABI territory if we have
multiple sequential bitfields in the structure where the compiler can
choose to combine them in some undefined way?
This follows the same pattern for bitfields seen in struct perf_event_attr.
I'm not sure we'll need lots of flags in our case though so perhaps it
would be fine to avoid the use of bitfields altogether here.
- Robert
>
>> -Chris
>>
>> --
>> Chris Wilson, Intel Open Source Technology Centre
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
