Re: [PATCH 09/11] drm/tegra: convert to struct drm_edid

[Jani Nikula <jani.nikula@intel.com>]

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
>> 

-- 
Jani Nikula, Intel