Re: [PATCH i-g-t v1 1/2] lib/igt_kms: Move backlight read, write to lib

["Reddy Guddati, Santhosh" <santhosh.reddy.guddati@intel.com>]

On 16-10-2024 09:59, Kandpal, Suraj wrote:
> 
> 
>> -----Original Message-----
>> From: Reddy Guddati, Santhosh <santhosh.reddy.guddati@intel.com>
>> Sent: Tuesday, October 15, 2024 9:54 PM
>> To: igt-dev@lists.freedesktop.org
>> Cc: Thasleem, Mohammed <mohammed.thasleem@intel.com>; Modem,
>> Bhanuprakash <bhanuprakash.modem@intel.com>; Kandpal, Suraj
>> <suraj.kandpal@intel.com>; Reddy Guddati, Santhosh
>> <santhosh.reddy.guddati@intel.com>
>> Subject: [PATCH i-g-t v1 1/2] lib/igt_kms: Move backlight read, write to lib
>>
>> move backlight_read and backlight_write functions from kms_pm_backlight
>> to library to re use in other tests.
>>
>> Signed-off-by: Santhosh Reddy Guddati
>> <santhosh.reddy.guddati@intel.com>
>> ---
>>   lib/igt_kms.c | 60
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/igt_kms.h | 13 +++++++++++
>>   2 files changed, 73 insertions(+)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c index bb35d4b82..c9c0ca1d3 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -7119,3 +7119,63 @@ void igt_reset_link_params(int drm_fd,
>> igt_output_t *output)
>>   	temp = drmModeGetConnector(drm_fd, output->config.connector-
>>> connector_id);
>>   	drmModeFreeConnector(temp);
>>   }
>> +
>> +/**
>> + * backlight_read:
>> + * @result:		Pointer to store the result
>> + * @fname:		Name of the file to read
>> + * @context:	Pointer to the context structure
>> + */
> 
> Now that this is a library function the naming should start with
> Igt_backlight_read same comment for the read.
> 
>> +int backlight_read(int *result, const char *fname,
>> +intel_backlight_context_t *context) {
>> +	int fd;
>> +	char full[PATH_MAX];
>> +	char dst[64];
>> +	int r, e;
>> +
>> +	igt_assert(snprintf(full, PATH_MAX, "%s/%s/%s",
>> INTEL_BACKLIGHT_PATH, context->path, fname)
>> +			    < PATH_MAX);
>> +	fd = open(full, O_RDONLY);
>> +	if (fd == -1)
>> +		return -errno;
>> +
>> +	r = read(fd, dst, sizeof(dst));
>> +	e = errno;
>> +	close(fd);
>> +
>> +	if (r < 0)
>> +		return -e;
>> +
>> +	errno = 0;
>> +	*result = strtol(dst, NULL, 10);
>> +	return errno;
>> +}
>> +
>> +/**
>> + * backlight_write:
>> + * @value:		Value to write
>> + * @fname:		Name of the file to write
>> + * @context:	Pointer to the context structure
>> + */
>> +int backlight_write(int value, const char *fname,
>> +intel_backlight_context_t *context) {
>> +	int fd;
>> +	char full[PATH_MAX];
>> +	char src[64];
>> +	int len;
>> +
>> +	igt_assert(snprintf(full, PATH_MAX, "%s/%s/%s",
>> INTEL_BACKLIGHT_PATH, context->path, fname)
> 
> Maybe add the path as a variable so that other may later on use it to
> Read their own backlight path since this is not isolated just to intel_backlight
> Test
> 
> Regards,
> Suraj Kandpal

 >> Thanks Suraj, I will update to use a new member in the existing 
struct intel_backlight_context_t to cache the backlight path.

Thanks,
Santhosh
> 
>> +		   < PATH_MAX);
>> +	fd = open(full, O_WRONLY);
>> +	if (fd == -1)
>> +		return -errno;
>> +
>> +	len = snprintf(src, sizeof(src), "%i", value);
>> +	len = write(fd, src, len);
>> +	close(fd);
>> +
>> +	if (len < 0)
>> +		return len;
>> +
>> +	return 0;
>> +}
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 2b26d2bbf..90b911e4d
>> 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -33,6 +33,7 @@
>>   #include <stdint.h>
>>   #include <stddef.h>
>>   #include <assert.h>
>> +#include <limits.h>
>>
>>   #include <xf86drmMode.h>
>>
>> @@ -513,6 +514,16 @@ typedef struct {
>>   	uint16_t tile_h_size, tile_v_size;
>>   } igt_tile_info_t;
>>
>> +#define INTEL_BACKLIGHT_PATH "/sys/class/backlight"
>> +
>> +/* Backlight context*/
>> +typedef struct {
>> +	int max;
>> +	int old;
>> +	igt_output_t *output;
>> +	char path[PATH_MAX];
>> +} intel_backlight_context_t;
>> +
>>   void igt_display_reset_outputs(igt_display_t *display);  void
>> igt_display_require(igt_display_t *display, int drm_fd);  void
>> igt_display_fini(igt_display_t *display); @@ -1253,5 +1264,7 @@ bool
>> igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t
>> *output  int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t
>> *output);  int igt_get_dp_pending_retrain(int drm_fd, igt_output_t
>> *output);  void igt_reset_link_params(int drm_fd, igt_output_t *output);
>> +int backlight_read(int *result, const char *fname,
>> +intel_backlight_context_t *context); int backlight_write(int value,
>> +const char *fname, intel_backlight_context_t *context);
>>
>>   #endif /* __IGT_KMS_H__ */
>> --
>> 2.34.1
>