On Thu, Aug 22, 2024 at 07:32:15PM +0300, Jani Nikula wrote:
> On Mon, 24 Jun 2024, Daniel Vetter <dan...@ffwll.ch> wrote:
> > On Tue, May 14, 2024 at 03:55:15PM +0300, Jani Nikula wrote:
> >> Prefer the struct drm_edid based functions for reading the EDID and
> >> updating the connector.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
> >> 
> >> ---
> >> 
> >> Cc: Thierry Reding <thierry.red...@gmail.com>
> >> Cc: Mikko Perttunen <mperttu...@nvidia.com>
> >> Cc: Jonathan Hunter <jonath...@nvidia.com>
> >> Cc: linux-te...@vger.kernel.org
> >> ---
> >>  drivers/gpu/drm/tegra/drm.h    |  2 +-
> >>  drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------
> >>  2 files changed, 18 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> >> index 682011166a8f..2f3781e04b0a 100644
> >> --- a/drivers/gpu/drm/tegra/drm.h
> >> +++ b/drivers/gpu/drm/tegra/drm.h
> >> @@ -133,7 +133,7 @@ struct tegra_output {
> >>    struct drm_bridge *bridge;
> >>    struct drm_panel *panel;
> >>    struct i2c_adapter *ddc;
> >> -  const struct edid *edid;
> >> +  const struct drm_edid *drm_edid;
> 
> I keep reviving old stuff, sorry.
> 
> Please take note here, removing the edid member.
> 
> >>    struct cec_notifier *cec;
> >>    unsigned int hpd_irq;
> >>    struct gpio_desc *hpd_gpio;
> >> diff --git a/drivers/gpu/drm/tegra/output.c 
> >> b/drivers/gpu/drm/tegra/output.c
> >> index 4da3c3d1abbc..e6b5863fec71 100644
> >> --- a/drivers/gpu/drm/tegra/output.c
> >> +++ b/drivers/gpu/drm/tegra/output.c
> >> @@ -21,7 +21,7 @@
> >>  int tegra_output_connector_get_modes(struct drm_connector *connector)
> >>  {
> >>    struct tegra_output *output = connector_to_output(connector);
> >> -  struct edid *edid = NULL;
> >> +  const struct drm_edid *drm_edid;
> >>    int err = 0;
> >>  
> >>    /*
> >> @@ -34,18 +34,17 @@ int tegra_output_connector_get_modes(struct 
> >> drm_connector *connector)
> >>                    return err;
> >>    }
> >>  
> >> -  if (output->edid)
> >> -          edid = kmemdup(output->edid, sizeof(*edid), GFP_KERNEL);
> >> +  if (output->drm_edid)
> >> +          drm_edid = drm_edid_dup(output->drm_edid);
> >>    else if (output->ddc)
> >> -          edid = drm_get_edid(connector, output->ddc);
> >> +          drm_edid = drm_edid_read_ddc(connector, output->ddc);
> >
> > Annoyingly the dsi output is the only one not using
> > drm_connector_init_with_ddc() despite that I think it could, so we can't
> > yet use drm_edid_read here ...
> >
> >>  
> >> -  cec_notifier_set_phys_addr_from_edid(output->cec, edid);
> >> -  drm_connector_update_edid_property(connector, edid);
> >> +  drm_edid_connector_update(connector, drm_edid);
> >> +  cec_notifier_set_phys_addr(output->cec,
> >> +                             
> >> connector->display_info.source_physical_address);
> >>  
> >> -  if (edid) {
> >> -          err = drm_add_edid_modes(connector, edid);
> >> -          kfree(edid);
> >> -  }
> >> +  err = drm_edid_connector_add_modes(connector);
> >> +  drm_edid_free(drm_edid);
> >>  
> >>    return err;
> >>  }
> >> @@ -98,6 +97,7 @@ static irqreturn_t hpd_irq(int irq, void *data)
> >>  int tegra_output_probe(struct tegra_output *output)
> >>  {
> >>    struct device_node *ddc, *panel;
> >> +  const void *edid;
> >>    unsigned long flags;
> >>    int err, size;
> >>  
> >> @@ -124,8 +124,6 @@ int tegra_output_probe(struct tegra_output *output)
> >>                    return PTR_ERR(output->panel);
> >>    }
> >>  
> >> -  output->edid = of_get_property(output->of_node, "nvidia,edid", &size);
> >> -
> >
> > I think you can/should also delete tegra_output.edid completely, since not
> > doing that freaked me out a bit until I checked everything :-) At least if
> > I didn't git grep the wrong regex.
> 
> I thought I am removing tegra_output.edid. What am I missing?

I'm sometimes just blind. rb without bikeshed applies, sorry for the
confusion.
-Sima

> 
> >
> > With that bikeshed addressed:
> >
> > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> 
> Thanks,
> Jani.
> 
> 
> >
> >>    ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
> >>    if (ddc) {
> >>            output->ddc = of_get_i2c_adapter_by_node(ddc);
> >> @@ -137,6 +135,9 @@ int tegra_output_probe(struct tegra_output *output)
> >>            }
> >>    }
> >>  
> >> +  edid = of_get_property(output->of_node, "nvidia,edid", &size);
> >> +  output->drm_edid = drm_edid_alloc(edid, size);
> >> +
> >>    output->hpd_gpio = devm_fwnode_gpiod_get(output->dev,
> >>                                    of_fwnode_handle(output->of_node),
> >>                                    "nvidia,hpd",
> >> @@ -187,6 +188,8 @@ int tegra_output_probe(struct tegra_output *output)
> >>    if (output->ddc)
> >>            i2c_put_adapter(output->ddc);
> >>  
> >> +  drm_edid_free(output->drm_edid);
> >> +
> >>    return err;
> >>  }
> >>  
> >> @@ -197,6 +200,8 @@ void tegra_output_remove(struct tegra_output *output)
> >>  
> >>    if (output->ddc)
> >>            i2c_put_adapter(output->ddc);
> >> +
> >> +  drm_edid_free(output->drm_edid);
> >>  }
> >>  
> >>  int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
> >> -- 
> >> 2.39.2
> >> 
> 
> -- 
> Jani Nikula, Intel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Reply via email to