Hi
Am 03.04.20 um 04:26 schrieb Xinliang Liu:
> Hi Tao,
>
> On Fri, 6 Mar 2020 at 11:44, Tian Tao <tiantao6@xxxxxxxxxxxxx> wrote:
>>
>> code cleanup for hibmc_drv_vdac.c, no actual function changes.
>>
>> Signed-off-by: Tian Tao <tiantao6@xxxxxxxxxxxxx>
>> Signed-off-by: Gong junjie <gongjunjie2@xxxxxxxxxx>
>> ---
>> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 49 ++++++++----------------
>> 1 file changed, 16 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> index 678ac2e..f0e6bb8 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
>> @@ -52,32 +52,6 @@ static const struct drm_connector_funcs hibmc_connector_funcs = {
>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> };
>>
>> -static struct drm_connector *
>> -hibmc_connector_init(struct hibmc_drm_private *priv)
>> -{
>> - struct drm_device *dev = priv->dev;
>> - struct drm_connector *connector;
>> - int ret;
>> -
>> - connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
>> - if (!connector) {
>> - DRM_ERROR("failed to alloc memory when init connector\n");
>> - return ERR_PTR(-ENOMEM);
>> - }
>> -
>> - ret = drm_connector_init(dev, connector,
>> - &hibmc_connector_funcs,
>> - DRM_MODE_CONNECTOR_VGA);
>> - if (ret) {
>> - DRM_ERROR("failed to init connector: %d\n", ret);
>> - return ERR_PTR(ret);
>> - }
>> - drm_connector_helper_add(connector,
>> - &hibmc_connector_helper_funcs);
>> -
>> - return connector;
>> -}
>> -
>> static void hibmc_encoder_mode_set(struct drm_encoder *encoder,
>> struct drm_display_mode *mode,
>> struct drm_display_mode *adj_mode)
>> @@ -109,13 +83,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>> struct drm_connector *connector;
>> int ret;
>>
>> - connector = hibmc_connector_init(priv);
>> - if (IS_ERR(connector)) {
>> - DRM_ERROR("failed to create connector: %ld\n",
>> - PTR_ERR(connector));
>> - return PTR_ERR(connector);
>> - }
>> -
>> encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
>> if (!encoder) {
>> DRM_ERROR("failed to alloc memory when init encoder\n");
>> @@ -131,6 +98,22 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
>> }
>>
>> drm_encoder_helper_add(encoder, &hibmc_encoder_helper_funcs);
>> + connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL);
>> + if (!connector) {
>> + DRM_ERROR("failed to alloc memory when init connector\n");
>> + return -ENOMEM;
>> + }
>> +
>> + ret = drm_connector_init(dev, connector,
>> + &hibmc_connector_funcs,
>> + DRM_MODE_CONNECTOR_VGA);
>> + if (ret) {
>> + DRM_ERROR("failed to init connector: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drm_connector_helper_add(connector, &hibmc_connector_helper_funcs);
>> + drm_connector_register(connector);
>
> You don't need to register a non-hotplug connector as it will be
> registered at drm_dev_register automatically.
>
> See function definition:
>
> 488 /**
> 489 * drm_connector_register - register a connector
> 490 * @connector: the connector to register
> 491 *
> 492 * Register userspace interfaces for a connector. Only call this
> for connectors
> 493 * which can be hotplugged after drm_dev_register() has been
> called already,
> 494 * e.g. DP MST connectors. All other connectors will be
> registered automatically
> 495 * when calling drm_dev_register().
> 496 *
> 497 * Returns:
> 498 * Zero on success, error code on failure.
> 499 */
> 500 int drm_connector_register(struct drm_connector *connector)
> 501 {
>
>
> Besides, I don't think this patch cleans much things.
True. Although the auto-kfree of devm_kzalloc() does not work correctly
with DRM and should be replaced.
I'd like to suggest to either allocate the connector as part of struct
hibmc_drm_private, or implement drmm_connector_create() on top of DRM's
new auto-cleanup functionality.
Best regards
Thomas
>
> -Xinliang
>
>> drm_connector_attach_encoder(connector, encoder);
>>
>> return 0;
>> --
>> 2.7.4
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
