Hi Daniel,
I'm honored to have your attention,and thanks for your advice.
I will finish the next version and submit it as soon as possible.

> > +
> > +     lcrtc->ldev = ldev;
> > +     lcrtc->reg_offset = index * REG_OFFSET;
> > +     lcrtc->cfg_reg = CFG_RESET;
> > +     lcrtc->crtc_id = index;
> > +
> > +     ret = loongson_plane_init(lcrtc);
> > +     if (ret)
> > +             goto free_lcrtc;
> > +
> > +     ret = drm_crtc_init_with_planes(ldev->dev, 
&lcrtc->base, &lcrtc->plane,
> > +                                     NULL, &loongson_crtc_funcs, 
NULL);
> 
> Please use the drmm_crtc version here and don't kzalloc yourself. That
> simplifies the cleanup (since atm you're just leaking this memory).
> 
> Also loongson hw looks a like like something which should use the simple
> kms helpers since you're embedding the single plane into your crtc struct:
> 
> 
https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#simple-kms-helper-reference
> 
> Also at that point your driver is probabyl small enough that single source
> file is the right thing, and you should move it into drivers/gpu/drm/tiny.
> It would be the first simple/tiny drm driver with 2 outputs.
> 
> Rule of thumb we have is that if it's below 1kloc, a single file and
> putting it into drm/tiny is best.
> 
> Cheers, Daniel

I will try use the drmm_crtc&drmm_encoder version, and consider use simple 
kms helpers.
But I'm not going to put it in drivers/gpu/drm/tiny, we still have some features
to work on and will support new graphics cards in the future.

> 
> > +     return 0;
> > +}
> > +
> > +static int loongson_drm_load(struct drm_device *dev)
> > +{
> > +     struct loongson_device *ldev;
> > +     int ret;
> > +
> > +     ldev = devm_kzalloc(dev->dev, sizeof(*ldev), GFP_KERNEL);
> 
> Please don't use devm_kzalloc here, but instead embedde the struct
> drm_device into your struct looongson_device.
> -Daniel

This has been changed and will be submitted in the next version.

> 
------------------------------
LiChenyang
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to