On Fri, Feb 17, 2017 at 05:20:54PM +0200, Jani Nikula wrote:
> Handle debugfs override edid and firmware edid at the low level to
> transparently and completely replace the real edid. Previously, we
> practically only used the modes from the override EDID, and none of the
> other data. This also prevents actual EDID reads when the EDID is to be
> overridden, but retains the DDC probe.
>
> Move firmware EDID loading from helper to core, as the functionality
> moves to lower level as well. This will result in a change of module
> parameter from drm_kms_helper.edid_firmware to drm.edid_firmware, which
> arguably makes more sense anyway.
>
> FIXME: validate override edid, deduplicate firmware edid validation.
>
> v2: move firmware loading to core
>
> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> ---
> drivers/gpu/drm/Kconfig | 2 +-
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_edid.c | 15 +++++++++++++++
> drivers/gpu/drm/drm_probe_helper.c | 19 +------------------
> 4 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 90bc65d07a35..f983ef60299c 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -101,7 +101,7 @@ config DRM_FBDEV_EMULATION
>
> config DRM_LOAD_EDID_FIRMWARE
> bool "Allow to specify an EDID data set instead of probing for it"
> - depends on DRM_KMS_HELPER
> + depends on DRM
> help
> Say Y here, if you want to use EDID data to be loaded from the
> /lib/firmware directory or one of the provided built-in
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 92de3991fa56..a10ac095608f 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -27,13 +27,13 @@ drm-$(CONFIG_DRM_PANEL) += drm_panel.o
> drm-$(CONFIG_OF) += drm_of.o
> drm-$(CONFIG_AGP) += drm_agpsupport.o
> drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
> +drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>
> drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
> drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
> drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
> drm_simple_kms_helper.o drm_modeset_helper.o
>
> -drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e1743ab276dc..4007998d5ce3 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1309,6 +1309,10 @@ static void connector_bad_edid(struct drm_connector *connector,
> * level, drivers must make all reasonable efforts to expose it as an I2C
> * adapter and use drm_get_edid() instead of abusing this function.
> *
> + * The EDID may be overridden using debugfs override_edid or firmare EDID
> + * (drm_load_edid_firmware()), in this priority order. Having either of them
> + * bypasses actual EDID reads.
> + *
> * Return: Pointer to valid EDID or NULL if we couldn't find any.
> */
> struct edid *drm_do_get_edid(struct drm_connector *connector,
> @@ -1318,6 +1322,17 @@ struct edid *drm_do_get_edid(struct drm_connector *connector,
> {
> int i, j = 0, valid_extensions = 0;
> u8 *edid, *new;
> + struct edid *override = NULL;
> +
> + if (connector->override_edid)
> + override = drm_edid_duplicate((const struct edid *)
> + connector->edid_blob_ptr->data);
> +
> + if (!override)
> + override = drm_load_edid_firmware(connector);
> +
> + if (!IS_ERR_OR_NULL(override))
> + return override;
>
> if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
> return NULL;
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 358957118ca9..871326cbc465 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -199,8 +199,6 @@ drm_connector_detect(struct drm_connector *connector, bool force)
> * drm_mode_probed_add(). New modes start their life with status as OK.
> * Modes are added from a single source using the following priority order.
> *
> - * - debugfs 'override_edid' (used for testing only)
> - * - firmware EDID (drm_load_edid_firmware())
> * - &drm_connector_helper_funcs.get_modes vfunc
> * - if the connector status is connector_status_connected, standard
> * VESA DMT modes up to 1024x768 are automatically added
> @@ -305,22 +303,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
> goto prune;
> }
>
> - if (connector->override_edid) {
> - struct edid *edid = (struct edid *) connector->edid_blob_ptr->data;
> -
> - count = drm_add_edid_modes(connector, edid);
> - drm_edid_to_eld(connector, edid);
> - } else {
> - struct edid *edid = drm_load_edid_firmware(connector);
> - if (!IS_ERR_OR_NULL(edid)) {
> - drm_mode_connector_update_edid_property(connector, edid);
> - count = drm_add_edid_modes(connector, edid);
> - drm_edid_to_eld(connector, edid);
> - kfree(edid);
> - }
> - if (count == 0)
> - count = (*connector_funcs->get_modes)(connector);
> - }
> + count = (*connector_funcs->get_modes)(connector);
What happens for drivers which cache the edid in their probe function, and
in their get_modes function only parse the already retrived edid? This
becomes fun in combination with the output forcing, where the connector
status forcing prevents ->detect from being run ...
I still think we can't handle override edid without also taking connector
forcing into account.
-Daniel
>
> if (count == 0 && connector->status == connector_status_connected)
> count = drm_add_modes_noedid(connector, 1024, 768);
> --
> 2.1.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
