From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Kumar, Shobhit" Subject: Re: [PATCH 1/3] drm/i915: Add get_config implementation for DSI encoder Date: Tue, 15 Jul 2014 17:54:07 +0530 Message-ID: <53C51D67.8000900@intel.com> References: <1405165643-13189-1-git-send-email-shobhit.kumar@intel.com> <1405165643-13189-2-git-send-email-shobhit.kumar@intel.com> <53C3EAEF.7090805@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 455E76E58A for ; Tue, 15 Jul 2014 05:24:10 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Jani Nikula , Daniel Vetter , intel-gfx List-Id: intel-gfx@lists.freedesktop.org On 7/14/2014 9:20 PM, Daniel Vetter wrote: > On Mon, Jul 14, 2014 at 4:36 PM, Kumar, Shobhit 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