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

["Kumar, Shobhit" <shobhit.kumar@intel.com>]

On 7/14/2014 9:20 PM, Daniel Vetter wrote:
> 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.

I just assumed that as we don't care about sync flags just suppress 
their check :) Thanks for pointing correct way of doing this. I will 
send the corrected patch

>
> 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 ;-)

Of-course. Thanks for clarifying.

Regards
Shobhit