| Message ID | 1556061656-1733-25-git-send-email-john.stultz@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Kirin driver cleanups to prep for Kirin960 support | expand |
On Wed, Apr 24, 2019 at 10:09 AM Sam Ravnborg <sam@ravnborg.org> wrote: > On Tue, Apr 23, 2019 at 04:20:55PM -0700, John Stultz wrote: > > static int kirin_drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > - struct drm_plane *plane) > > + struct drm_plane *plane, > > + const struct kirin_drm_data *driver_data) > > Indent looks wrong here. > .. > > ret = drm_universal_plane_init(dev, &kplane->base, 1, > > + driver_data->plane_funcs, > > + driver_data->channel_formats, > > + driver_data->channel_formats_cnt, > > + NULL, type, NULL); > Indent looks wrong here. Thanks! I've now fixed those up. > I missed where ade_driver_data came from. > This looks an extra patch to intoduce driver_data, > that maybe should be merged with an earlier version? I'm not sure I'm following you here. Can you clarify a bit more? thanks -john
On Wed, Apr 24, 2019 at 2:15 PM Sam Ravnborg <sam@ravnborg.org> wrote: > > > I missed where ade_driver_data came from. > > > This looks an extra patch to intoduce driver_data, > > > that maybe should be merged with an earlier version? > > > > I'm not sure I'm following you here. Can you clarify a bit more? > > So I looked at this a bit more - and got the bigger picture in place > again. > > driver_data is assigned using the lookup done at probe() time. > For now this is just assigned to ade_driver_data as this is the > only option. > So an indirection via driver_date or calling ade_driver_data > direct is the same. > And you have several patches where you migrate to use driver_data > rather than calling ade_driver_data direct. > It confused me that the patch introducing the lookup at probe() > came before all call sites were migrated to use driver_data. > But I get it now so it is fine. > > Maybe a few words in the commit log like: > > This patch refactor to call functions via driver_data, > rather than hardcoding them via ade_driver_data. > This is doen so we later can assing another stucture to > driver_data to support other chips. Sounds good! I'll integrate this into the change log. > PS. I did not complain about your spelling mistakes in the > changelog. I have a similar (or worse) keyboard from a spelling > point of view. Oh yes, a deficiency of mine. Good reminder I should run through the logs w/ the spell checker. Again, I appreciate the feedback! thanks -john
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c index 71671f8..876e25b 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c @@ -572,7 +572,8 @@ static const struct drm_crtc_funcs ade_crtc_funcs = { }; static int kirin_drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, - struct drm_plane *plane) + struct drm_plane *plane, + const struct kirin_drm_data *driver_data) { struct device_node *port; int ret; @@ -589,13 +590,13 @@ static int kirin_drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, crtc->port = port; ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL, - ade_driver_data.crtc_funcs, NULL); + driver_data->crtc_funcs, NULL); if (ret) { DRM_ERROR("failed to init crtc.\n"); return ret; } - drm_crtc_helper_add(crtc, ade_driver_data.crtc_helper_funcs); + drm_crtc_helper_add(crtc, driver_data->crtc_helper_funcs); return 0; } @@ -894,21 +895,22 @@ static struct drm_plane_funcs ade_plane_funcs = { static int kirin_drm_plane_init(struct drm_device *dev, struct kirin_plane *kplane, - enum drm_plane_type type) + enum drm_plane_type type, + const struct kirin_drm_data *driver_data) { int ret = 0; ret = drm_universal_plane_init(dev, &kplane->base, 1, - ade_driver_data.plane_funcs, - ade_driver_data.channel_formats, - ade_driver_data.channel_formats_cnt, - NULL, type, NULL); + driver_data->plane_funcs, + driver_data->channel_formats, + driver_data->channel_formats_cnt, + NULL, type, NULL); if (ret) { DRM_ERROR("fail to init plane, ch=%d\n", kplane->ch); return ret; } - drm_plane_helper_add(&kplane->base, ade_driver_data.plane_helper_funcs); + drm_plane_helper_add(&kplane->base, driver_data->plane_helper_funcs); return 0; } @@ -1024,14 +1026,15 @@ static int ade_drm_init(struct platform_device *pdev) else type = DRM_PLANE_TYPE_OVERLAY; - ret = kirin_drm_plane_init(dev, kplane, type); + ret = kirin_drm_plane_init(dev, kplane, type, &ade_driver_data); if (ret) return ret; } /* crtc init */ ret = kirin_drm_crtc_init(dev, &kcrtc->base, - &ade->planes[ade_driver_data.prim_plane].base); + &ade->planes[ade_driver_data.prim_plane].base, + &ade_driver_data); if (ret) return ret;