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

On Mon, Sep 18, 2017 at 09:20:03PM +0300, Jani Nikula wrote:

Add drm_kms_helper.edid_firmware module parameter with param ops hooks to set drm.edid_firmware instead, for backwards compatibility.

Suggested-by: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Abdiel Janulgue abdiel.janulgue@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Jani Nikula jani.nikula@intel.com

---

drivers/gpu/drm/drm_edid_load.c | 16 ++++++++++++++++ drivers/gpu/drm/drm_kms_helper_common.c | 28 ++++++++++++++++++++++++++++ include/drm/drm_edid.h | 2 ++ 3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 1c0495acf341..a4915099aaa9 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -31,6 +31,22 @@ 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 */ +int __drm_set_edid_firmware_path(const char *path) +{

• scnprintf(edid_firmware, sizeof(edid_firmware), "%s", path);
• return 0;

+} +EXPORT_SYMBOL(__drm_set_edid_firmware_path);

+/* Use only for backward compatibility with drm_kms_helper.edid_firmware */ +int __drm_get_edid_firmware_path(char *buf, size_t bufsize) +{

• return scnprintf(buf, bufsize, "%s", edid_firmware);

I guess these could just use strlcpy() or something.

+} +EXPORT_SYMBOL(__drm_get_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..5051c3aa4d5d 100644 --- a/drivers/gpu/drm/drm_kms_helper_common.c +++ b/drivers/gpu/drm/drm_kms_helper_common.c @@ -26,6 +26,7 @@ */

#include <linux/module.h> +#include <drm/drmP.h>

#include "drm_crtc_helper_internal.h"

@@ -33,6 +34,33 @@ 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)

+/* Backward compatibility for drm_kms_helper.edid_firmware */ +static int edid_firmware_set(const char *val, const struct kernel_param *kp) +{

• DRM_NOTE("drm_kms_firmware.edid_firmware is deprecated, please use drm.edid_firmware intead.\n");
• return __drm_set_edid_firmware_path(val);

+}

+static int edid_firmware_get(char *buffer, const struct kernel_param *kp) +{

• return __drm_get_edid_firmware_path(buffer, PAGE_SIZE);

+}

+const struct kernel_param_ops edid_firmware_ops = {

• .set = edid_firmware_set,
• .get = edid_firmware_get,

+};

+module_param_cb(edid_firmware, &edid_firmware_ops, NULL, 0644);

Do we want the __parm_check thing here? Looks like it's just some kind of compile time type check though so not critical by any means.

Otherwise looks technically correct so Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

+__MODULE_PARM_TYPE(edid_firmware, "charp"); +MODULE_PARM_DESC(edid_firmware,

•  "DEPRECATED. Use drm.edid_firmware module parameter instead.");
  

+#endif

static int __init drm_kms_helper_init(void) { int ret; diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 1e1908a6b1d6..6f35909b8add 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -341,6 +341,8 @@ 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); +int __drm_set_edid_firmware_path(const char *path); +int __drm_get_edid_firmware_path(char *buf, size_t bufsize); #else static inline struct edid * drm_load_edid_firmware(struct drm_connector *connector) -- 2.11.0