[PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Sean Paul]

On Tue, Nov 4, 2014 at 9:50 AM, Thierry Reding <thierry.reding at gmail.com> wrote:
> On Mon, Nov 03, 2014 at 12:18:30PM -0500, Sean Paul wrote:
>> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding at gmail.com> wrote:
>> > From: Thierry Reding <treding at nvidia.com>
>> >
>> > The output is already enabled in .dpms(), doing it in .mode_set() too
>> > can cause noticeable flicker.
>> >
>>
>> I think this should be coupled with "drm/tegra: DPMS off/on in encoder
>> prepare/commit" that I sent earlier this week. Without it, the driver
>> can get into a state where connector status is on, but the output is
>> disabled.
>
> I'm not sure I exactly understand which problem that patch fixes, but
> I'll give it some testing to see if it doesn't break anything.
>

I'll try to explain it more clearly.

The problem occurs when userspace does set_property(dpms_on), then modeset.

When the set_property ioctl to set dpms = on is called, the tegra
driver goes through drm_helper_connector_dpms(). At this point,
because the connector has not participated in a modeset,
connector->encoder == NULL. In drm_helper_connector_dpms(), we set
connector->dpms = DRM_MODE_DPMS_ON, and skip everything else because
!encoder && !crtc.

When modeset is called, we go through drm_crtc_helper_set_config().
This function will populate connector->encoder with the appropriate
encoder and call drm_crtc_helper_set_mode(), which goes through the
prepare/mode_set/commit calls. Finally, drm_crtc_helper_set_config()
iterates through the connectors involved in the modeset and calls
their dpms() func. In our case, as above, we go through
drm_helper_connector_dpms(), but it just exits early because mode ==
connector->dpms.

So we end up in a state where connector->dpms == DRM_MODE_DPMS_ON, and
the output, as well as any connected panel, has never been enabled.
The reason is that they're only enabled in encoder_funcs->dpms(),
which is never called in the above scenario.

I hope that makes more sense :-)

Sean


> Thierry