[RFC v2 1/2] drm/exynos: Add Picture Processor framework

[Marek Szyprowski]

Hi Emil,

On 2017-05-08 15:43, Emil Velikov wrote:
> Hi Marek,
>
> A couple of small nitpicks from UAPI POV.

Thanks for your comments!

> On 8 May 2017 at 10:11, Marek Szyprowski <m.szyprowski at samsung.com> wrote:
>
>> --- a/include/uapi/drm/exynos_drm.h
>> +++ b/include/uapi/drm/exynos_drm.h
>> +struct drm_exynos_pp_get_res {
>> +       __u64 pp_id_ptr;
>> +       __u32 count_pps;
> Add __u32 pad - sizeof(struct ...) should be multiple of sizeof(__u64).

ok

>> +struct drm_exynos_pp_get {
>> +       __u32 pp_id;
>> +       __u32 capabilities;
>> +
>> +       __u32 src_format_count;
>> +       __u32 dst_format_count;
>> +       __u32 params_count;
>> +       __u32 reserved1;
>> +
> Replace with __u32 flags; so that you can extend the struct as applicable.

ok

>> +       __u64 src_format_type_ptr;
>> +       __u64 dst_format_type_ptr;
>> +       __u64 params_ptr;
>> +       __u64 reserved2;
> And now you can drop this piece.
>
>
>> +struct drm_exynos_pp_commit {
>> +       __u32 id;
>> +       __u32 flags;
>> +       __u32 params_count;
>> +       __u32 reserved;
> Why the reserved here - flags should help you extend as needed.

To ensure that the structure will be a multiple of sizeof(u64).

>> +       __u64 param_ids_ptr;
>> +       __u64 param_values_ptr;
>> +       __u64 user_data;
> Unused user_data?

Nope. User_data provided here will be reported back to userspace in the 
struct
drm_exynos_pp_event, so we cannot drop it.

>> +struct drm_exynos_pp_event {
>> +       struct drm_event        base;
>> +       __u64                   user_data;
> Unused user_data?

See comment above.

>> +       __u32                   tv_sec;
>> +       __u32                   tv_usec;
>> +       __u32                   pp_id;
>> +       __u32                   sequence;
>> +       __u64                   reserved;
> Drop in favour of flags?

u64 flags? Probably u32 flags + u32 reserved would make a bit more sense to
keep structure a multiple of sizeof(u64).

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland