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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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 <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>
> 
> ---
> 
> 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 <linux/module.h>
> +#include <drm/drmP.h>
>  
>  #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