Received-SPF: pass (google.com: domain of linux-kernel+bounces-92883-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240304195214.14563-1-hsinyi@chromium.org> <20240304195214.14563-3-hsinyi@chromium.org>
 <87a5nd4tsg.fsf@intel.com> <CAJMQK-j4wGah=szyUW53hu-v6Q4QjgR7WMLKnspoFaO9oPfaQw@mail.gmail.com>
 <874jdl4k01.fsf@intel.com> <CAJMQK-iWHoh6s-hkcNULzZLjMg9UnTuWfjaJ=YfnHU3sQ1NBEg@mail.gmail.com>
 <CAD=FV=UOhTGnhtc9gOQ5C_aAdgVcB+K7NL9RGm4umunF91Wkpg@mail.gmail.com> <87y1ax2iu1.fsf@intel.com>
In-Reply-To: <87y1ax2iu1.fsf@intel.com>
From: Doug Anderson <dianders@chromium.org>
Date: Tue, 5 Mar 2024 11:25:20 -0800
Message-ID: <CAD=FV=WzLLeEw-b0Kug-Pm-9EYm7eHvmukEUJ8VHnu-4YY3WNQ@mail.gmail.com>
Subject: Re: [PATCH v3 2/4] drm/edid: Add a function to check monitor string
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Hsin-Yi Wang <hsinyi@chromium.org>, Neil Armstrong <neil.armstrong@linaro.org>, 
	Jessica Zhang <quic_jesszhan@quicinc.com>, Sam Ravnborg <sam@ravnborg.org>, 
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, 
	Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, 
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, 
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Mar 5, 2024 at 12:17=E2=80=AFAM Jani Nikula <jani.nikula@linux.inte=
l.com> wrote:
>
> On Mon, 04 Mar 2024, Doug Anderson <dianders@chromium.org> wrote:
> > Hi,
> >
> > On Mon, Mar 4, 2024 at 4:19=E2=80=AFPM Hsin-Yi Wang <hsinyi@chromium.or=
g> wrote:
> >>
> >> > > Probably change to u32 drm_edid_get_panel_id(const struct drm_edid
> >> > > *);? Given that we still need to parse id from
> >> > > drm_edid_read_base_block().
> >> >
> >> > No, we no longer need to parse the id outside of drm_edid.c. You'll =
have
> >> > the id's in panel code in the form of struct drm_edid_ident (or
> >> > whatever), and use the match function to see if the opaque drm_edid
> >> > matches.
> >> >
> >> drm_panel prints the panel_id info on whether the panel is detected or=
 not.
> >> https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/gpu/drm/panel=
/panel-edp.c#L792
> >>
> >> Is it okay to remove this information?
> >
> > Hmmm, I guess it also is exported via debugfs, actually. See
> > detected_panel_show() in panel-edp.c. We probably don't want to remove
> > that...
>
> You currently print the information via panel->detected_panel, which is
> a struct edp_panel_entry *. That doesn't change. It'll be slightly
> restructured to contain a struct drm_edid_ident, which will not be an
> opaque type.

Hmm. As Hsin-Yi pointed out to me offline. Somehow we'll need to get
the actual panel ID out. Right now in panel-edp.c we have:

dev_warn(dev,
  "Unknown panel %s %#06x, using conservative timings\n",
  vend, product_id);

Where "vend" and "product_id" come from the panel ID of a panel that
we didn't recognize. For instance:

  Unknown panel BOE 0x0731, using conservative timings

We need to still be able to print this message for unrecognized
panels. Then when we see field reports including this message we know
that somehow we ended up shipping an unrecognized panel.

Any suggestions on what abstraction you'd like to see to enable us to
print that message if everything is opaque?

-Doug