Re: [PATCH v3 2/2] drm: add backwards compatibility support for drm_kms_helper.edid_firmware

[Jani Nikula]

On Fri, 15 Sep 2017, Ville Syrjälä  wrote:
> Hmm. Wouldn't you just have to have custom kernel_param_ops and
> that's about it? Seems like that could be a bit cleaner.

Here's a shot at that. Completely untested, but seems like this should
do the trick, and is less complex than I anticipated.

BR,
Jani.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/2] drm: add backwards compatibility support for drm_kms_helper.edid_firmware

[Ville Syrjälä]

On Tue, Sep 12, 2017 at 11:19:27AM +0300, Jani Nikula wrote:
> If drm_kms_helper.edid_firmware module parameter is set at
> drm_kms_helper probe time, update the new drm.edid_firmware parameter
> for backwards compatibility.
> 
> The drm_kms_helper.edid_firmware is now read-only in sysfs to prevent
> runtime updates.
> 
> This is a sort of easy middle ground; adding a full runtime support via
> /sys/module/drm_kms_helper/parameters/edid_firmware would be possible,
> but considerably more and uglier code.

Hmm. Wouldn't you just have to have custom kernel_param_ops and
that's about it? Seems like that could be a bit cleaner.

> 
> Cc: Abdiel Janulgue 
> Cc: Daniel Vetter 
> Cc: Ville Syrjälä 
> Signed-off-by: Jani Nikula 
> 
> ---
> 
> I'm wondering if the change from drm_kms_helper.edid_firmware to
> drm.edid_firmware is enough of an ABI breakage to warrant something like
> this patch.
> ---
>  drivers/gpu/drm/drm_edid_load.c |  7 +++
>  drivers/gpu/drm/drm_kms_helper_common.c | 19 +++
>  include/drm/drm_edid.h  |  2 ++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index 1c0495acf341..a94005529cd4 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -31,6 +31,13 @@ module_param_string(edid_firmware, edid_firmware, 
> sizeof(edid_firmware), 0644);
>  MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use specified EDID 
> blob "
>   "from built-in data or /lib/firmware instead. ");
>  
> +/* Use only for backward compatibility with drm_kms_helper.edid_firmware */
> +void __drm_set_edid_firmware_path(const char *path)
> +{
> + strlcpy(edid_firmware, path, sizeof(edid_firmware));
> +}
> +EXPORT_SYMBOL(__drm_set_edid_firmware_path);
> +
>  #define GENERIC_EDIDS 6
>  static const char * const generic_edid_name[GENERIC_EDIDS] = {
>   "edid/800x600.bin",
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c 
> b/drivers/gpu/drm/drm_kms_helper_common.c
> index 6e35a56a6102..6130199a83b6 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -26,6 +26,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include "drm_crtc_helper_internal.h"
>  
> @@ -33,10 +34,28 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
>  MODULE_DESCRIPTION("DRM KMS helper");
>  MODULE_LICENSE("GPL and additional rights");
>  
> +#if IS_ENABLED(CONFIG_DRM_LOAD_EDID_FIRMWARE)
> +static char edid_firmware[PATH_MAX];
> +module_param_string(edid_firmware, edid_firmware, sizeof(edid_firmware), 
> 0444);
> +MODULE_PARM_DESC(edid_firmware, "DEPRECATED. Use drm.edid_firmware 
> instead.");
> +static inline void legacy_set_edid_firmare_path(void)
> +{
> + if (edid_firmware[0]) {
> + DRM_NOTE("drm_kms_firmware.edid_firmware is deprecated, "
> +  "please use drm.edid_firmware intead.\n");
> + __drm_set_edid_firmware_path(edid_firmware);
> + }
> +}
> +#else
> +static inline void legacy_set_edid_firmare_path(void) {}
> +#endif
> +
>  static int __init drm_kms_helper_init(void)
>  {
>   int ret;
>  
> + legacy_set_edid_firmare_path();
> +
>   /* Call init functions from specific kms helpers here */
>   ret = drm_fb_helper_modinit();
>   if (ret < 0)
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 1e1908a6b1d6..cdfb793255f0 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -341,12 +341,14 @@ int drm_av_sync_delay(struct drm_connector *connector,
>  
>  #ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
>  struct edid *drm_load_edid_firmware(struct drm_connector *connector);
> +void __drm_set_edid_firmware_path(const char *path);
>  #else
>  static inline struct edid *
>  drm_load_edid_firmware(struct drm_connector *connector)
>  {
>   return ERR_PTR(-ENOENT);
>  }
> +static inline void __drm_set_edid_firmware_path(const char *path) {}
>  #endif
>  
>  int
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 2/2] drm: add backwards compatibility support for drm_kms_helper.edid_firmware

[Jani Nikula]

If drm_kms_helper.edid_firmware module parameter is set at
drm_kms_helper probe time, update the new drm.edid_firmware parameter
for backwards compatibility.

The drm_kms_helper.edid_firmware is now read-only in sysfs to prevent
runtime updates.

This is a sort of easy middle ground; adding a full runtime support via
/sys/module/drm_kms_helper/parameters/edid_firmware would be possible,
but considerably more and uglier code.

Cc: Abdiel Janulgue 
Cc: Daniel Vetter 
Cc: Ville Syrjälä 
Signed-off-by: Jani Nikula 

---

I'm wondering if the change from drm_kms_helper.edid_firmware to
drm.edid_firmware is enough of an ABI breakage to warrant something like
this patch.
---
 drivers/gpu/drm/drm_edid_load.c |  7 +++
 drivers/gpu/drm/drm_kms_helper_common.c | 19 +++
 include/drm/drm_edid.h  |  2 ++
 3 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 1c0495acf341..a94005529cd4 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -31,6 +31,13 @@ module_param_string(edid_firmware, edid_firmware, 
sizeof(edid_firmware), 0644);
 MODULE_PARM_DESC(edid_firmware, "Do not probe monitor, use specified EDID blob 
"
"from built-in data or /lib/firmware instead. ");
 
+/* Use only for backward compatibility with drm_kms_helper.edid_firmware */
+void __drm_set_edid_firmware_path(const char *path)
+{
+   strlcpy(edid_firmware, path, sizeof(edid_firmware));
+}
+EXPORT_SYMBOL(__drm_set_edid_firmware_path);
+
 #define GENERIC_EDIDS 6
 static const char * const generic_edid_name[GENERIC_EDIDS] = {
"edid/800x600.bin",
diff --git a/drivers/gpu/drm/drm_kms_helper_common.c 
b/drivers/gpu/drm/drm_kms_helper_common.c
index 6e35a56a6102..6130199a83b6 100644
--- a/drivers/gpu/drm/drm_kms_helper_common.c
+++ b/drivers/gpu/drm/drm_kms_helper_common.c
@@ -26,6 +26,7 @@
  */
 
 #include 
+#include 
 
 #include "drm_crtc_helper_internal.h"
 
@@ -33,10 +34,28 @@ MODULE_AUTHOR("David Airlie, Jesse Barnes");
 MODULE_DESCRIPTION("DRM KMS helper");
 MODULE_LICENSE("GPL and additional rights");
 
+#if IS_ENABLED(CONFIG_DRM_LOAD_EDID_FIRMWARE)
+static char edid_firmware[PATH_MAX];
+module_param_string(edid_firmware, edid_firmware, sizeof(edid_firmware), 0444);
+MODULE_PARM_DESC(edid_firmware, "DEPRECATED. Use drm.edid_firmware instead.");
+static inline void legacy_set_edid_firmare_path(void)
+{
+   if (edid_firmware[0]) {
+   DRM_NOTE("drm_kms_firmware.edid_firmware is deprecated, "
+"please use drm.edid_firmware intead.\n");
+   __drm_set_edid_firmware_path(edid_firmware);
+   }
+}
+#else
+static inline void legacy_set_edid_firmare_path(void) {}
+#endif
+
 static int __init drm_kms_helper_init(void)
 {
int ret;
 
+   legacy_set_edid_firmare_path();
+
/* Call init functions from specific kms helpers here */
ret = drm_fb_helper_modinit();
if (ret < 0)
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 1e1908a6b1d6..cdfb793255f0 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -341,12 +341,14 @@ int drm_av_sync_delay(struct drm_connector *connector,
 
 #ifdef CONFIG_DRM_LOAD_EDID_FIRMWARE
 struct edid *drm_load_edid_firmware(struct drm_connector *connector);
+void __drm_set_edid_firmware_path(const char *path);
 #else
 static inline struct edid *
 drm_load_edid_firmware(struct drm_connector *connector)
 {
return ERR_PTR(-ENOENT);
 }
+static inline void __drm_set_edid_firmware_path(const char *path) {}
 #endif
 
 int
-- 
2.11.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel