| Message ID | e037667dfc325925bdeabff539df1e708d7c3c64.1715691257.git.jani.nikula@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
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.nikula@intel.com> > > --- > > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Mikko Perttunen <mperttunen@nvidia.com> > Cc: Jonathan Hunter <jonathanh@nvidia.com> > Cc: linux-tegra@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; > 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. With that bikeshed addressed: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > 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 >
On Mon, 24 Jun 2024, Daniel Vetter <daniel@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.nikula@intel.com> >> >> --- >> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> Cc: Mikko Perttunen <mperttunen@nvidia.com> >> Cc: Jonathan Hunter <jonathanh@nvidia.com> >> Cc: linux-tegra@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? > > With that bikeshed addressed: > > Reviewed-by: Daniel Vetter <daniel.vetter@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 >>
On Thu, Aug 22, 2024 at 07:32:15PM +0300, Jani Nikula wrote: > On Mon, 24 Jun 2024, Daniel Vetter <daniel@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.nikula@intel.com> > >> > >> --- > >> > >> Cc: Thierry Reding <thierry.reding@gmail.com> > >> Cc: Mikko Perttunen <mperttunen@nvidia.com> > >> Cc: Jonathan Hunter <jonathanh@nvidia.com> > >> Cc: linux-tegra@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.vetter@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
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; 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); - 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); - 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)
Prefer the struct drm_edid based functions for reading the EDID and updating the connector. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Mikko Perttunen <mperttunen@nvidia.com> Cc: Jonathan Hunter <jonathanh@nvidia.com> Cc: linux-tegra@vger.kernel.org --- drivers/gpu/drm/tegra/drm.h | 2 +- drivers/gpu/drm/tegra/output.c | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)