Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Mon, 16 Oct 2023 10:46:03 +0200
From:   Alvin =?utf-8?Q?=C5=A0ipraga?= <alvin@pqrs.dk>
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     Andrzej Hajda <andrzej.hajda@intel.com>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Robert Foss <rfoss@kernel.org>,
        Jonas Karlman <jonas@kwiboo.se>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Alvin =?utf-8?Q?=C5=A0ipraga?= <alsi@bang-olufsen.dk>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>
Subject: Re: [PATCH] drm: bridge: adv7511: get edid in hpd_work to update CEC
 phys address
Message-ID: <7dyh2eggk6ezag4ksmtac7b5gh3vzty3tf5547iyzlugbmwqap@rtssnbq57xuu>
References: <20231014-adv7511-cec-edid-v1-1-a58ceae0b57e@bang-olufsen.dk>
 <20231016081749.GE23177@pendragon.ideasonboard.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231016081749.GE23177@pendragon.ideasonboard.com>
Precedence: bulk

Hi Laurent,

On Mon, Oct 16, 2023 at 11:17:49AM +0300, Laurent Pinchart wrote:
> Hi Alvin,
> 
> Thank you for the patch.
> 
> CC'ing Hans Verkuil, to review the CEC side.
> 
> On Sat, Oct 14, 2023 at 09:43:01PM +0200, Alvin Šipraga wrote:
> > From: Alvin Šipraga <alsi@bang-olufsen.dk>
> > 
> > The adv7511 driver is solely responsible for setting the physical
> > address of its CEC adapter. To do this, it must read the EDID. However,
> > EDID is only read when either the drm_bridge_funcs :: get_edid or
> > drm_connector_helper_funcs :: get_modes ops are called. Without loss of
> > generality, it cannot be assumed that these ops are called when a sink
> > gets attached. Therefore there exist scenarios in which the CEC physical
> > address will be invalid (f.f.f.f), rendering the CEC adapter inoperable.
> > 
> > Address this problem by always fetching the EDID in the HPD work when we
> > detect a connection. The CEC physical address is set in the process.
> > 
> > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> > ---
> > Pardon the insertion of the ugly adv7511_get_edid() prototype, but I did
> > not want to clobber git history by rearranging a bunch of functions. If
> > this is the preferred approach I will happily re-spin the patch.
> 
> There's nothing wrong in rearranging functions, it is actually preferred
> to adding forward declarations. You can submit a set of two patches, one
> to reorder the functions, and then a second one to fix the problem. This
> makes review easier by isolating the refactoring with no functional
> change from the functional changes.

Alright, good to know. I will wait a while for Hans to take a look before
sending a v2 series with two patches. Thanks!

Kind regards,
Alvin