Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Tue, 10 Oct 2023 21:02:42 +0200
From:   Mehdi Djait <mehdi.djait@bootlin.com>
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     mchehab@kernel.org, heiko@sntech.de, hverkuil-cisco@xs4all.nl,
        krzysztof.kozlowski+dt@linaro.org, robh+dt@kernel.org,
        conor+dt@kernel.org, linux-media@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        thomas.petazzoni@bootlin.com, alexandre.belloni@bootlin.com,
        maxime.chevallier@bootlin.com, paul.kocialkowski@bootlin.com
Subject: Re: [PATCH v6 0/3] media: i2c: Introduce driver for the TW9900 video
 decoder
Message-ID: <ZSWf0iVvBoZGbjAx@Host-002>
References: <cover.1696608809.git.mehdi.djait@bootlin.com>
 <20231009022122.GH5121@pendragon.ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231009022122.GH5121@pendragon.ideasonboard.com>
Precedence: bulk

Hi Laurent,

On Mon, Oct 09, 2023 at 05:21:22AM +0300, Laurent Pinchart wrote:
> On Fri, Oct 06, 2023 at 06:25:27PM +0200, Mehdi Djait wrote:
> > Hello everyone,
> > 
> > This series is based on the fifth iteration of the series introducing the
> > tw9900 driver: sent 29 Dec 2020 [1]
> > 
> > This is the version 6 of the series adding support for the Techwell
> > TW9900 multi standard decoder. It's a pretty simple decoder compared to
> > the TW9910, since it doesn't have a built-in scaler/crop engine.
> > 
> > Changes v5 => v6:
> > - dropped .skip_top and .field in the supported_modes
> > - added error handling for the i2c writes/reads
> > - added the colorimetry information to fill_fmt
> > - removed pm_runtime
> 
> It's not very nice to keep the chip powered up all the time :-(
> 

I agree 100% I tried to make it work with pm_runtime but I faced many
issues. I don't know if this is due to my lack of experience but here is
the situation when I enable pm_runtime:

I get most of the time wrong values when calling g_input_status to check
if I have a signal or not. 

To do that I read the 0x01 – Chip Status Register I (STATUS1) and check
the BIT(6): HLOCK:
	- 1 = Horizontal sync PLL is locked to the incoming video source.
	- 0 = Horizontal sync PLL is not locked.

To make the g_input_status work with pm_runtime I had to add a 300
msleep after power ON! Which is a huge delay.

I also face issues with the standard detection...
So I decided to drop it for this first version of the driver.

--
Kind Regards
Mehdi Djait