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

Dave Airlie airlied at gmail.com
Thu Sep 28 04:32:09 UTC 2017


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.


More information about the amd-gfx mailing list