Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type

Maarten Lankhorst
2017-10-27T14:05:36Z

Op 27-10-17 om 16:03 schreef Ville Syrjälä:
> On Fri, Oct 27, 2017 at 03:53:34PM +0200, Maarten Lankhorst wrote:
>> Op 27-10-17 om 14:44 schreef Ville Syrjälä:
>>> On Fri, Oct 27, 2017 at 02:05:38PM +0200, Maarten Lankhorst wrote:
>>>> Op 19-10-17 om 15:37 schreef Ville Syrjala:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> Currently the DDI encoder->type will change at runtime depending on
>>>>> what kind of hotplugs we've processed. That's quite bad since we can't
>>>>> really trust that that current value of encoder->type actually matches
>>>>> the type of signal we're trying to drive through it.
>>>>>
>>>>> Let's eliminate that problem by declaring that non-eDP DDI port will
>>>>> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
>>>>> can no longer try to distinguish DP vs. HDMI based on encoder->type.
>>>>> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
>>>>> there's a bunch of code that relies on that value to identify eDP
>>>>> encoders.
>>>>>
>>>>> We'll introduce a new encoder .compute_output_type() hook. This allows
>>>>> us to compute the full output_types before any encoder .compute_config()
>>>>> hooks get called, thus those hooks can rely on output_types being
>>>>> correct, which is useful for cloning on oldr platforms. For now we'll
>>>>> just look at the connector type and pick the correct mode based on that.
>>>>> In the future the new hook could be used to implement dynamic switching
>>>>> between LS and PCON modes for LSPCON.
>>>>>
>>>>> TODO: maybe make .get_config() populate output_types rather than doing
>>>>> the default encoder->type thing in caller and then undoing it for
>>>>> DDI in .get_config().
>>>>>
>>>>> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
>>>>> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
>>>>> v4: Rebase
>>>>> v5: Populate output_types in .get_config() rather than in the caller
>>>> Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
>>>> patch towards removing the use?
>>> We don't want to remove it.
>> Ok, but could that still be a separate preparation patch? That would make it easier to review. :)
> Not sure which part you mean here. Moving the 'output_types |= whatever'
> into .get_config()? Hmm. Yeah that should be easy to split out. I guess
> it won't even make the DDI case any more broken that it already is since
> even currently encoder->type might change between the modeset compute
> phase and .get_config(). And it's going to be followed closely by the
> actual fix anyway.
>
Yes, that part. It should work even without the UNKNOWN->DDI change afaics.