diff mbox series

[2/4] drm/edid: add drm_edid_print_product_id()

Message ID dcefe0474d74f818e250677982b32c767b466110.1711015462.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/edid & drm/i915: vendor and product id logging improvements | expand

Commit Message

Jani Nikula March 21, 2024, 10:05 a.m. UTC
Add a function to print a decoded EDID vendor and product id to a drm
printer, optinally with the raw data.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |  3 +++
 2 files changed, 38 insertions(+)

Comments

Ville Syrjälä April 8, 2024, 6:05 p.m. UTC | #1
On Thu, Mar 21, 2024 at 12:05:10PM +0200, Jani Nikula wrote:
> Add a function to print a decoded EDID vendor and product id to a drm
> printer, optinally with the raw data.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |  3 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 626a0e24e66a..198986f0eb8b 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -29,6 +29,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/byteorder/generic.h>
>  #include <linux/cec.h>
>  #include <linux/hdmi.h>
>  #include <linux/i2c.h>
> @@ -2771,6 +2772,40 @@ void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>  }
>  EXPORT_SYMBOL(drm_edid_get_product_id);
>  
> +/**
> + * drm_edid_print_product_id - Print decoded product id to printer
> + * @p: drm printer
> + * @id: EDID product id
> + * @raw: If true, also print the raw hex
> + */
> +void drm_edid_print_product_id(struct drm_printer *p,
> +			       const struct drm_edid_product_id *id, bool raw)
> +{
> +	u16 mfg_id = id->manufacturer_name[0] << 8 | id->manufacturer_name[1];
> +	char *date;
> +	char vend[4];
> +
> +	drm_edid_decode_mfg_id(mfg_id, vend);
> +
> +	if (id->week_of_manufacture == 0xff)

Didn't realize this had a loaded meaning. Maybe we should also
skip the week printout if week==0? Otherwise people might think
week==0 means the first week.

> +		date = kasprintf(GFP_KERNEL, "model year: %d",
> +				 id->year_of_manufacture + 1990);
> +	else
> +		date = kasprintf(GFP_KERNEL, "week: %d, year of manufacture: %d",

The "week: %d" part feels a bit left out here. Maybe this should be
formatted as "week/year of manufacture: %d/%d"? 

Not sure I like the kasprintf(). Maybe use an on-stack buffer?

> +				 id->week_of_manufacture,
> +				 id->year_of_manufacture + 1990);
> +
> +	drm_printf(p, "manufacturer name: %s, product code: %u, serial number: %u, %s\n",
> +		   vend, le16_to_cpu(id->product_code),
> +		   le32_to_cpu(id->serial_number), date ?: "");
> +
> +	if (raw)
> +		drm_printf(p, "raw product id: %*ph\n", (int)sizeof(*id), id);
> +
> +	kfree(date);
> +}
> +EXPORT_SYMBOL(drm_edid_print_product_id);
> +
>  /**
>   * drm_edid_get_panel_id - Get a panel's ID from EDID
>   * @drm_edid: EDID that contains panel ID.
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 7911a2f8a672..c763ba1a0bbd 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -30,6 +30,7 @@ struct drm_connector;
>  struct drm_device;
>  struct drm_display_mode;
>  struct drm_edid;
> +struct drm_printer;
>  struct hdmi_avi_infoframe;
>  struct hdmi_vendor_infoframe;
>  struct i2c_adapter;
> @@ -481,6 +482,8 @@ int drm_edid_connector_add_modes(struct drm_connector *connector);
>  bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>  void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>  			     struct drm_edid_product_id *id);
> +void drm_edid_print_product_id(struct drm_printer *p,
> +			       const struct drm_edid_product_id *id, bool raw);
>  
>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>  				  int ext_id, int *ext_index);
> -- 
> 2.39.2
Jani Nikula April 9, 2024, 9:41 a.m. UTC | #2
On Mon, 08 Apr 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Mar 21, 2024 at 12:05:10PM +0200, Jani Nikula wrote:
>> Add a function to print a decoded EDID vendor and product id to a drm
>> printer, optinally with the raw data.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 35 +++++++++++++++++++++++++++++++++++
>>  include/drm/drm_edid.h     |  3 +++
>>  2 files changed, 38 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 626a0e24e66a..198986f0eb8b 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -29,6 +29,7 @@
>>   */
>>  
>>  #include <linux/bitfield.h>
>> +#include <linux/byteorder/generic.h>
>>  #include <linux/cec.h>
>>  #include <linux/hdmi.h>
>>  #include <linux/i2c.h>
>> @@ -2771,6 +2772,40 @@ void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>>  }
>>  EXPORT_SYMBOL(drm_edid_get_product_id);
>>  
>> +/**
>> + * drm_edid_print_product_id - Print decoded product id to printer
>> + * @p: drm printer
>> + * @id: EDID product id
>> + * @raw: If true, also print the raw hex
>> + */
>> +void drm_edid_print_product_id(struct drm_printer *p,
>> +			       const struct drm_edid_product_id *id, bool raw)
>> +{
>> +	u16 mfg_id = id->manufacturer_name[0] << 8 | id->manufacturer_name[1];
>> +	char *date;
>> +	char vend[4];
>> +
>> +	drm_edid_decode_mfg_id(mfg_id, vend);
>> +
>> +	if (id->week_of_manufacture == 0xff)
>
> Didn't realize this had a loaded meaning. Maybe we should also
> skip the week printout if week==0? Otherwise people might think
> week==0 means the first week.

Agreed.

>
>> +		date = kasprintf(GFP_KERNEL, "model year: %d",
>> +				 id->year_of_manufacture + 1990);
>> +	else
>> +		date = kasprintf(GFP_KERNEL, "week: %d, year of manufacture: %d",
>
> The "week: %d" part feels a bit left out here. Maybe this should be
> formatted as "week/year of manufacture: %d/%d"? 

Agreed.

> Not sure I like the kasprintf(). Maybe use an on-stack buffer?

Refactored using struct seq_buf and a helper function; v2 out shortly.

BR,
Jani.

>
>> +				 id->week_of_manufacture,
>> +				 id->year_of_manufacture + 1990);
>> +
>> +	drm_printf(p, "manufacturer name: %s, product code: %u, serial number: %u, %s\n",
>> +		   vend, le16_to_cpu(id->product_code),
>> +		   le32_to_cpu(id->serial_number), date ?: "");
>> +
>> +	if (raw)
>> +		drm_printf(p, "raw product id: %*ph\n", (int)sizeof(*id), id);
>> +
>> +	kfree(date);
>> +}
>> +EXPORT_SYMBOL(drm_edid_print_product_id);
>> +
>>  /**
>>   * drm_edid_get_panel_id - Get a panel's ID from EDID
>>   * @drm_edid: EDID that contains panel ID.
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index 7911a2f8a672..c763ba1a0bbd 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -30,6 +30,7 @@ struct drm_connector;
>>  struct drm_device;
>>  struct drm_display_mode;
>>  struct drm_edid;
>> +struct drm_printer;
>>  struct hdmi_avi_infoframe;
>>  struct hdmi_vendor_infoframe;
>>  struct i2c_adapter;
>> @@ -481,6 +482,8 @@ int drm_edid_connector_add_modes(struct drm_connector *connector);
>>  bool drm_edid_is_digital(const struct drm_edid *drm_edid);
>>  void drm_edid_get_product_id(const struct drm_edid *drm_edid,
>>  			     struct drm_edid_product_id *id);
>> +void drm_edid_print_product_id(struct drm_printer *p,
>> +			       const struct drm_edid_product_id *id, bool raw);
>>  
>>  const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
>>  				  int ext_id, int *ext_index);
>> -- 
>> 2.39.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 626a0e24e66a..198986f0eb8b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -29,6 +29,7 @@ 
  */
 
 #include <linux/bitfield.h>
+#include <linux/byteorder/generic.h>
 #include <linux/cec.h>
 #include <linux/hdmi.h>
 #include <linux/i2c.h>
@@ -2771,6 +2772,40 @@  void drm_edid_get_product_id(const struct drm_edid *drm_edid,
 }
 EXPORT_SYMBOL(drm_edid_get_product_id);
 
+/**
+ * drm_edid_print_product_id - Print decoded product id to printer
+ * @p: drm printer
+ * @id: EDID product id
+ * @raw: If true, also print the raw hex
+ */
+void drm_edid_print_product_id(struct drm_printer *p,
+			       const struct drm_edid_product_id *id, bool raw)
+{
+	u16 mfg_id = id->manufacturer_name[0] << 8 | id->manufacturer_name[1];
+	char *date;
+	char vend[4];
+
+	drm_edid_decode_mfg_id(mfg_id, vend);
+
+	if (id->week_of_manufacture == 0xff)
+		date = kasprintf(GFP_KERNEL, "model year: %d",
+				 id->year_of_manufacture + 1990);
+	else
+		date = kasprintf(GFP_KERNEL, "week: %d, year of manufacture: %d",
+				 id->week_of_manufacture,
+				 id->year_of_manufacture + 1990);
+
+	drm_printf(p, "manufacturer name: %s, product code: %u, serial number: %u, %s\n",
+		   vend, le16_to_cpu(id->product_code),
+		   le32_to_cpu(id->serial_number), date ?: "");
+
+	if (raw)
+		drm_printf(p, "raw product id: %*ph\n", (int)sizeof(*id), id);
+
+	kfree(date);
+}
+EXPORT_SYMBOL(drm_edid_print_product_id);
+
 /**
  * drm_edid_get_panel_id - Get a panel's ID from EDID
  * @drm_edid: EDID that contains panel ID.
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 7911a2f8a672..c763ba1a0bbd 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -30,6 +30,7 @@  struct drm_connector;
 struct drm_device;
 struct drm_display_mode;
 struct drm_edid;
+struct drm_printer;
 struct hdmi_avi_infoframe;
 struct hdmi_vendor_infoframe;
 struct i2c_adapter;
@@ -481,6 +482,8 @@  int drm_edid_connector_add_modes(struct drm_connector *connector);
 bool drm_edid_is_digital(const struct drm_edid *drm_edid);
 void drm_edid_get_product_id(const struct drm_edid *drm_edid,
 			     struct drm_edid_product_id *id);
+void drm_edid_print_product_id(struct drm_printer *p,
+			       const struct drm_edid_product_id *id, bool raw);
 
 const u8 *drm_find_edid_extension(const struct drm_edid *drm_edid,
 				  int ext_id, int *ext_index);