Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder

Daniel Vetter
2014-07-14T15:50:27Z

On Mon, Jul 14, 2014 at 4:36 PM, Kumar, Shobhit <shobhit.kumar@intel.com> wrote:
>>> /* XXX: read flags, set to adjusted_mode */
>>> + pipe_config->quirks = 1;
>>
>>
>> Nack. First you need to use one of the symbolic quirk definitions
>> (there's a bunch of them). Second this needs a comment why exactly we
>> need the quirk (which really only should be used if there's no way to
>> read a given piece of state back from the hw).
>
>
> Okay, in MIPI we have sync events going as short packets. In that case I
> think it should be okay to use PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS ?

Well it depends. From a quick look it seems like the current dsi code
doesn't care at all about sync flags. In that case you should
normalize the sync flags of the adjusted mode in the compute_config
callback to 0 and not set them in the get_hw_state function. We do
that already for e.g. tv encoder outputs.

The quirk flag should only be used if we do set the sync modes but
somehow can't read it back. The only case is sdvo where some encoders
(in violation of the spec) don't support the flag readback. But that
case needs a big comment explaining why.

The goal here isn't to shut up the hw cross checker but to actually
make it useful ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch