On Mon, 24 Jun 2024, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxx>
>>
>> ---
>>
>> Cc: Thierry Reding <thierry.reding@xxxxxxxxx>
>> Cc: Mikko Perttunen <mperttunen@xxxxxxxxxx>
>> Cc: Jonathan Hunter <jonathanh@xxxxxxxxxx>
>> Cc: linux-tegra@xxxxxxxxxxxxxxx
>> ---
>> 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@xxxxxxxx>
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
