Re: [PATCH 08/11] drm/msm/dp: switch to struct drm_edid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 20 May 2024 at 15:25, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
>
> On Sun, 19 May 2024, Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote:
> > On Tue, May 14, 2024 at 03:55:14PM +0300, Jani Nikula wrote:
> >> Prefer the struct drm_edid based functions for reading the EDID and
> >> updating the connector.
> >>
> >> Simplify the flow by updating the EDID property when the EDID is read
> >> instead of at .get_modes.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> >>
> >> ---
> >
> > The patch looks good to me, I'd like to hear an opinion from Doug (added
> > to CC).
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> Thanks!
>
> > What is the merge strategy for the series? Do you plan to pick up all
> > the patches to drm-misc or should we pick up individual patches into
> > driver trees?
>
> I think all of the patches here are connected in theme, but
> independent. Either way is fine by me.
>
> >
> >
> >>
> >> Cc: Rob Clark <robdclark@xxxxxxxxx>
> >> Cc: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> >> Cc: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> >> Cc: Sean Paul <sean@xxxxxxxxxx>
> >> Cc: Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx>
> >> Cc: linux-arm-msm@xxxxxxxxxxxxxxx
> >> Cc: freedreno@xxxxxxxxxxxxxxxxxxxxx
> >> ---
> >>  drivers/gpu/drm/msm/dp/dp_display.c | 11 +++----
> >>  drivers/gpu/drm/msm/dp/dp_panel.c   | 47 +++++++++--------------------
> >>  drivers/gpu/drm/msm/dp/dp_panel.h   |  2 +-
> >>  3 files changed, 20 insertions(+), 40 deletions(-)
> >
> > [skipped]
> >
> >> @@ -249,10 +228,12 @@ void dp_panel_handle_sink_request(struct dp_panel *dp_panel)
> >>      panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
> >>
> >>      if (panel->link->sink_request & DP_TEST_LINK_EDID_READ) {
> >> +            /* FIXME: get rid of drm_edid_raw() */
> >
> > The code here can get use of something like drm_edid_smth_checksum().
> > 'Something', because I could not come up with the word that would make
> > it clear that it is the declared checksum instead of the actual /
> > computed one.
>
> This is an annoying one, to be honest, and linked to the historical fact
> that we filter some EDID blocks that have an incorrect checksum.

It is a part of the DP test suite if I remember correctly.

>
> (Some blocks, yes. We don't filter all blocks, because there are some
> nasty docks out there that modify the CTA block but fail to update the
> checksum, and filtering the CTA blocks would render the display
> useless. So we accept CTA blocks with incorrect checksums. But reject
> others. Yay.)
>
> IMO the real fix would be to stop mucking with the EDID, and just expose
> it to userspace, warts and all. We could still ignore the EDID blocks
> with incorrect checksum while using it ourselves if we want to. And with
> that, we could just have a function that checks the last EDID block's
> checksum, and stop using this ->real_edid_checksum thing.
>
> Anyway, yes, we could add the function already.
>
> BR,
> Jani.
>
> >
> >> +            const struct edid *edid = drm_edid_raw(dp_panel->drm_edid);
> >>              u8 checksum;
> >>
> >> -            if (dp_panel->edid)
> >> -                    checksum = dp_panel_get_edid_checksum(dp_panel->edid);
> >> +            if (edid)
> >> +                    checksum = dp_panel_get_edid_checksum(edid);
> >>              else
> >>                      checksum = dp_panel->connector->real_edid_checksum;
> >>
>
> --
> Jani Nikula, Intel



-- 
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux