[Intel-gfx] [PATCH 2/2] drm/i915: Set the digital port encoder personality during modeset
Paulo Zanoni
przanoni at gmail.com
Thu Dec 5 13:38:48 CET 2013
2013/12/3 Daniel Vetter <daniel at ffwll.ch>:
> On Tue, Dec 03, 2013 at 03:36:20PM -0200, Paulo Zanoni wrote:
>> From: Damien Lespiau <damien.lespiau at intel.com>
>>
>> The ->detect() vfunc of connectors is usually responsible for setting the
>> encoder type on intel_digital_ports when a hotplug event happens.
>>
>> However we sometimes want to force a modeset on a specific connector:
>> - the user can ask the SETCRTC ioctl to use a connector that isn't
>> marked as connected (because we never received a hotplug event for it).
>> This can be also used in tests to do a modeset without the need of a
>> plugged-in monitor.
>> - the command line video= option can be used to force modesets, eg.:
>> video=HDMI-A-1:1024x768e
>>
>> So, before we try to do anything with the DDI encoder as part of a modeset,
>> we need to ensure that the personality of the encoder matches the selected
>> connector.
>>
>> v2: Made by Paulo:
>> - Return false if we have more than one connector.
>> - WARN in case it's not eDP/DP/HDMI
>> - Don't break error strings in pieces
>> - Change the "status == connected" message from DRM_ERROR to
>> DRM_DEBUG_KMS
>> - Don't store+use the old encoder->type at compute_config
>> - Add bugzilla reference
>> - Add testcase reference
>>
>> Testcase: igt/kms_setmode/clone-exclusive-crtc
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=68463
>> Tested-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 96 ++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 93 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index c8382f5..879fbe8 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1442,18 +1442,108 @@ static void intel_ddi_destroy(struct drm_encoder *encoder)
>> intel_dp_encoder_destroy(encoder);
>> }
>>
>> +/*
>> + * The ->detect() vfunc of connectors is usually responsible for setting the
>> + * encoder type on intel_digital_ports when a hotplug event happens.
>> + *
>> + * However we sometimes want to force a modeset on a specific connector:
>> + * - the user can ask the SETCRTC ioctl to use a connector that isn't marked
>> + * as connected (because we never received a hotplug event for it).
>> + * This can be also used in tests to do a modeset without the need of a
>> + * plugged-in monitor.
>> + * - the command line video= option can be used to force modesets, eg.:
>> + * video=HDMI-A-1:1024x768e
>> + *
>> + * So, before we try to do anything with the DDI encoder as part of a modeset,
>> + * we need to ensure that the personality of the encoder matches the selected
>> + * connector.
>> + */
>> +static bool
>> +intel_ddi_ensure_encoder_type(struct intel_encoder *encoder)
>> +{
>> + struct drm_device *dev = encoder->base.dev;
>> + struct intel_connector *connector;
>> + int connectors = 0;
>> +
>> + list_for_each_entry(connector, &dev->mode_config.connector_list,
>> + base.head) {
>> + int connector_type, old_encoder_type, new_encoder_type;
>> + int port;
>> +
>> + if (connector->new_encoder != encoder)
>> + continue;
>> +
>> + connectors++;
>> + if (connectors > 1) {
>> + DRM_DEBUG_KMS("Can't set modes for 2 connectors on the same DDI encoder\n");
>> + return false;
>> + }
>
> This part should be moved into it's own patch and also moved into the
> modeset core - sdvo encoders suffer from the same issue. Even better, if
> we put that in the common code we can reject modeset attempts even before
> we've started to kill the old config, which is always what we want.
> Otherwise we'll never get to the great promised land where modesetting
> configurations can be checked non-destructively with the atomic ioctl.
See "[PATCH] drm/i915: don't set modes for 2 connectors on the same
encoder": http://lists.freedesktop.org/archives/intel-gfx/2013-November/036771.html
. The implementation is based on something you posted to pastebin when
we were discussing this problem a few months ago.
Even with that patch applied, I think we should keep this current path
as-is since it runs before the code that checks for 2 encoders.
>
> With change I'd simply call the function ddi_set_encoder_type or
> something.
>
> Cheers, Daniel
>> +
>> + connector_type = connector->base.connector_type;
>> + old_encoder_type = encoder->type;
>> + switch (connector_type) {
>> + case DRM_MODE_CONNECTOR_HDMIA:
>> + case DRM_MODE_CONNECTOR_HDMIB:
>> + new_encoder_type = INTEL_OUTPUT_HDMI;
>> + break;
>> + case DRM_MODE_CONNECTOR_DisplayPort:
>> + new_encoder_type = INTEL_OUTPUT_DISPLAYPORT;
>> + break;
>> + case DRM_MODE_CONNECTOR_eDP:
>> + continue;
>> + default:
>> + WARN(1, "DRM connector type %d\n", connector_type);
>> + continue;
>> + }
>> +
>> + if (old_encoder_type == new_encoder_type)
>> + continue;
>> +
>> + port = intel_ddi_get_encoder_port(encoder);
>> +
>> + if (old_encoder_type == INTEL_OUTPUT_EDP) {
>> + DRM_ERROR("Can't change DDI %c personality to %s, it's an eDP DDI\n",
>> + port_name(port),
>> + intel_output_name(new_encoder_type));
>> + return false;
>> + }
>> +
>> + if (connector->base.status == connector_status_connected) {
>> + DRM_DEBUG_KMS("Can't change DDI %c personality to %s, it has a connected %s device\n",
>> + port_name(port),
>> + intel_output_name(new_encoder_type),
>> + intel_output_name(old_encoder_type));
>> + return false;
>> + }
>> +
>> + DRM_DEBUG_KMS("Changing DDI %c from %s to %s\n",
>> + port_name(port),
>> + intel_output_name(old_encoder_type),
>> + intel_output_name(new_encoder_type));
>> +
>> + encoder->type = new_encoder_type;
>> + }
>> +
>> + return true;
>> +}
>> +
>> static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>> struct intel_crtc_config *pipe_config)
>> {
>> - int type = encoder->type;
>> int port = intel_ddi_get_encoder_port(encoder);
>> + int ret;
>> +
>> + ret = intel_ddi_ensure_encoder_type(encoder);
>> + if (!ret)
>> + return false;
>>
>> - WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
>> + WARN(encoder->type == INTEL_OUTPUT_UNKNOWN,
>> + "compute_config() on unknown output!\n");
>>
>> if (port == PORT_A)
>> pipe_config->cpu_transcoder = TRANSCODER_EDP;
>>
>> - if (type == INTEL_OUTPUT_HDMI)
>> + if (encoder->type == INTEL_OUTPUT_HDMI)
>> return intel_hdmi_compute_config(encoder, pipe_config);
>> else
>> return intel_dp_compute_config(encoder, pipe_config);
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
More information about the Intel-gfx
mailing list