[PATCH 03/77] drm/amd/display: add display write back(DWB)

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

 



On 1 September 2017 at 04:07, Harry Wentland <harry.wentland at amd.com> wrote:
> From: Duke Du <Duke.Du at amd.com>

I happened to look at dwb stuff in passing today.

there is a bunch of dwb API that look to be never called, and a bunch
of over engineered interfaces.

Comments inline:

> +struct dwbc_funcs {
> +       bool (*get_caps)(struct dwbc *dwbc, struct dwb_caps *caps);
> +
> +       bool (*enable)(struct dwbc *dwbc);
> +
> +       bool (*disable)(struct dwbc *dwbc);
> +
> +       bool (*get_status)(struct dwbc *dwbc, struct dwb_status *status);
> +
> +       bool (*dump_frame)(struct dwbc *dwbc, struct dwb_frame_info *frame_info,
> +               unsigned char *luma_buffer, unsigned char *chroma_buffer,
> +               unsigned char *dest_luma_buffer, unsigned char *dest_chroma_buffer);
> +
> +       bool (*set_basic_settings)(struct dwbc *dwbc,
> +               const struct dwb_basic_settings *basic_settings);
> +
> +       bool (*get_basic_settings)(struct dwbc *dwbc,
> +               struct dwb_basic_settings *basic_settings);
> +
> +       bool (*set_advanced_settings)(struct dwbc *dwbc,
> +               const struct dwb_advanced_settings *advanced_settings);
> +
> +       bool (*get_advanced_settings)(struct dwbc *dwbc,
> +               struct dwb_advanced_settings *advanced_settings);
> +
> +       bool (*reset_advanced_settings)(struct dwbc *dwbc);
> +};

So a fair few of these return bool for no reason, and accept NULL as a
parameter,
returning true or false. this is all pointless, apart from the fact
that no code currently
calls these, calling APIs and passing NULL should be caught with code
inspection,
not adding error paths that nobody will ever test.

If your API expects something assert on it being there, don't add
true/false return values.

Dave.


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

  Powered by Linux