Re: [igt-dev] [PATCH i-g-t v2 5/6] lib/dsc: Add helpers for VDSC output format debugfs entry

Kamil Konieczny
2023-02-03T12:39:49Z

Hi Swati,

On 2023-02-02 at 12:51:20 +0530, Swati Sharma wrote:
> Helper functions are added for VDSC output format debugfs entry.
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
> lib/igt_dsc.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/igt_dsc.h | 5 +++++
> lib/igt_kms.h | 6 ++++++
> 3 files changed, 60 insertions(+)
>
> diff --git a/lib/igt_dsc.c b/lib/igt_dsc.c
> index 64dd6b29a..1f6d33291 100644
> --- a/lib/igt_dsc.c
> +++ b/lib/igt_dsc.c
> @@ -134,3 +134,52 @@ int igt_get_dsc_debugfs_fd(int drmfd, char *connector_name)
>
> return openat(igt_debugfs_dir(drmfd), file_name, O_WRONLY);
> }
> +
> +/*
--- ^
Descriptions should start with two stars, so it should look like:

/**
* igt_is_dsc_output_format_supported_by_sink:

> + * igt_is_dsc_output_format_supported_by_sink:
> + * @drmfd: A drm file descriptor
> + * @connector_name: Name of the libdrm connector we're going to use
> + * @output_format: Output format
> + *
> + * Returns: True if DSC output format is supported for the given connector,
> + * false otherwise.
> + */
> +bool igt_is_dsc_output_format_supported_by_sink(int drmfd, char *connector_name,
> + enum dsc_output_format output_format)
> +{
> + const char *check_str = "OUTPUTFORMATNOTFOUND";
> +
> + switch(output_format) {
--------------^
Put space after "switch" and before opening "(":
switch (output_format) {

Please use checkpatch script from Linux kernel sources, it can help
you catch such errors.

> + case DSC_FORMAT_RGB:
> + check_str = "RGB: yes";
> + break;
> + case DSC_FORMAT_YCBCR420:
> + check_str = "YCBCR420: yes";
> + break;
> + case DSC_FORMAT_YCBCR444:
> + check_str = "YCBCR444: yes";
> + break;
> + default:
> + break;
> + }
> +
> + return check_dsc_debugfs(drmfd, connector_name, check_str);
> +}
> +
> +/*
--- ^
Same here, two stars.

Regards,
Kamil

> + * igt_force_dsc_output_format:
> + * @drmfd: A drm file descriptor
> + * @connector_name: Name of the libdrm connector we're going to use
> + * @output_format: Output format
> + *
> + * Returns: 0 on success or negative error code, in case of failure.
> + */
> +int igt_force_dsc_output_format(int drmfd, char *connector_name,
> + enum dsc_output_format output_format)
> +{
> + char buf[20] = {0};
> +
> + sprintf(buf, "%d", output_format);
> +
> + return write_dsc_debugfs(drmfd, connector_name, "i915_dsc_output_format", buf);
> +}
> diff --git a/lib/igt_dsc.h b/lib/igt_dsc.h
> index 291c2cdea..9608aad44 100644
> --- a/lib/igt_dsc.h
> +++ b/lib/igt_dsc.h
> @@ -7,6 +7,7 @@
> #define IGT_DSC_H
>
> #include "igt_fb.h"
> +#include "igt_kms.h"
>
> bool igt_is_dsc_supported(int drmfd, char *connector_name);
> bool igt_is_fec_supported(int drmfd, char *connector_name);
> @@ -15,5 +16,9 @@ bool igt_is_force_dsc_enabled(int drmfd, char *connector_name);
> int igt_force_dsc_enable(int drmfd, char *connector_name);
> int igt_force_dsc_enable_bpc(int drmfd, char *connector_name, int bpc);
> int igt_get_dsc_debugfs_fd(int drmfd, char *connector_name);
> +bool igt_is_dsc_output_format_supported_by_sink(int drmfd, char *connector_name,
> + enum dsc_output_format output_format);
> +int igt_force_dsc_output_format(int drmfd, char *connector_name,
> + enum dsc_output_format output_format);
>
> #endif
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index be5482e08..9f5bef170 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -106,6 +106,12 @@ enum igt_custom_edid_type {
> */
> #define kmstest_port_name(port) ((port) + 'A')
>
> +enum dsc_output_format {
> + DSC_FORMAT_RGB = 0,
> + DSC_FORMAT_YCBCR420,
> + DSC_FORMAT_YCBCR444
> +};
> +
> const char *kmstest_encoder_type_str(int type);
> const char *kmstest_connector_status_str(int status);
> const char *kmstest_connector_type_str(int type);
> --
> 2.25.1
>