Re: [PATCH 2/4] drm/edid: add drm_edid_print_product_id()
On Mon, 08 Apr 2024, Ville Syrjälä 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
>> ---
>> 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
>> +#include
>> #include
>> #include
>> #include
>> @@ -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
--
Jani Nikula, Intel
Re: [PATCH 2/4] drm/edid: add drm_edid_print_product_id()
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
> ---
> 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
> +#include
> #include
> #include
> #include
> @@ -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
--
Ville Syrjälä
Intel
[PATCH 2/4] drm/edid: add drm_edid_print_product_id()
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
---
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
+#include
#include
#include
#include
@@ -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);
--
2.39.2
