| Message ID | 1556061656-1733-12-git-send-email-john.stultz@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Kirin driver cleanups to prep for Kirin960 support | expand |
Hi John. On Tue, Apr 23, 2019 at 04:20:42PM -0700, John Stultz wrote: > From: Xu YiPing <xuyiping@hisilicon.com> > > As part of refactoring the kirin driver to better support > different hardware revisions, this patch moves some shared > structures and helpers to the common kirin_drm_drv.h > > These structures will later used by both kirin620 and > future kirin960 driver > > Cc: Xinliang Liu <z.liuxinliang@hisilicon.com> > Cc: Rongrong Zou <zourongrong@gmail.com> > Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com> > Cc: Chen Feng <puck.chen@hisilicon.com> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: dri-devel <dri-devel@lists.freedesktop.org> > Signed-off-by: Xu YiPing <xuyiping@hisilicon.com> > [jstultz: reworded commit message] > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 27 ++----------------------- > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 24 ++++++++++++++++++++++ > 2 files changed, 26 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c > index f54cf99..69604ad 100644 > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c > @@ -38,12 +38,6 @@ > #define OUT_OVLY ADE_OVLY2 /* output overlay compositor */ > #define ADE_DEBUG 1 > > -#define to_kirin_crtc(crtc) \ > - container_of(crtc, struct kirin_crtc, base) > - > -#define to_kirin_plane(plane) \ > - container_of(plane, struct kirin_plane, base) > - > > struct ade_hw_ctx { > void __iomem *base; > @@ -59,18 +53,6 @@ struct ade_hw_ctx { > struct drm_crtc *crtc; > }; > > -struct kirin_crtc { > - struct drm_crtc base; > - void *hw_ctx; > - bool enable; > -}; > - > -struct kirin_plane { > - struct drm_plane base; > - void *hw_ctx; > - u32 ch; > -}; > - > struct ade_data { > struct kirin_crtc crtc; > struct kirin_plane planes[ADE_CH_NUM]; > @@ -78,12 +60,7 @@ struct ade_data { > }; This struct: > /* ade-format info: */ > -struct ade_format { > - u32 pixel_format; > - enum ade_fb_format ade_format; > -}; > - > -static const struct ade_format ade_formats[] = { > +static const struct kirin_format ade_formats[] = { > /* 16bpp RGB: */ > { DRM_FORMAT_RGB565, ADE_RGB_565 }, > { DRM_FORMAT_BGR565, ADE_BGR_565 }, > @@ -127,7 +104,7 @@ static u32 ade_get_format(u32 pixel_format) > > for (i = 0; i < ARRAY_SIZE(ade_formats); i++) > if (ade_formats[i].pixel_format == pixel_format) > - return ade_formats[i].ade_format; > + return ade_formats[i].hw_format; > > /* not found */ > DRM_ERROR("Not found pixel format!!fourcc_format= %d\n", > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h > index ad027d1..b6626f5 100644 > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h > @@ -13,6 +13,30 @@ > > #define MAX_CRTC 2 > > +#define to_kirin_crtc(crtc) \ > + container_of(crtc, struct kirin_crtc, base) > + > +#define to_kirin_plane(plane) \ > + container_of(plane, struct kirin_plane, base) > + > +/* kirin-format translate table */ > +struct kirin_format { > + u32 pixel_format; > + u32 hw_format; > +}; Is renamed. The member hw_format is renamed and no longer uses an enum. (The sole user of this enum type). These changes are not included in the changelog - should they be part of this patch? And also the change from enum to u32 is not understood. Sam
On Wed, Apr 24, 2019 at 9:50 AM Sam Ravnborg <sam@ravnborg.org> wrote: > On Tue, Apr 23, 2019 at 04:20:42PM -0700, John Stultz wrote: > > This struct: > > /* ade-format info: */ > > -struct ade_format { > > - u32 pixel_format; > > - enum ade_fb_format ade_format; > > -}; > > - > > -static const struct ade_format ade_formats[] = { > > +static const struct kirin_format ade_formats[] = { > > /* 16bpp RGB: */ > > { DRM_FORMAT_RGB565, ADE_RGB_565 }, > > { DRM_FORMAT_BGR565, ADE_BGR_565 }, ... > > + > > +/* kirin-format translate table */ > > +struct kirin_format { > > + u32 pixel_format; > > + u32 hw_format; > > +}; > Is renamed. > The member hw_format is renamed and no longer uses an enum. > (The sole user of this enum type). So the enum values are still used, but yes, the type here shifts. > These changes are not included in the changelog - should they be part of > this patch? > And also the change from enum to u32 is not understood. So the intent is to be able to share the kirin_format structure between both the kirin620 support and kirin960, where as the ade_fb_format enum values are tied to the kirin620, the kirin960 has dpe_fb_formats enum which has different values. So the u32 hw_format value is just the generic storage for enumerated format types of either device. So its just a map from generic pixel_format definition -> hardware specific value for that format. I'll try to make that change more clear in the commit message, but if you have ideas for a simpler or cleaner way to do the same, let me know. 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 f54cf99..69604ad 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c @@ -38,12 +38,6 @@ #define OUT_OVLY ADE_OVLY2 /* output overlay compositor */ #define ADE_DEBUG 1 -#define to_kirin_crtc(crtc) \ - container_of(crtc, struct kirin_crtc, base) - -#define to_kirin_plane(plane) \ - container_of(plane, struct kirin_plane, base) - struct ade_hw_ctx { void __iomem *base; @@ -59,18 +53,6 @@ struct ade_hw_ctx { struct drm_crtc *crtc; }; -struct kirin_crtc { - struct drm_crtc base; - void *hw_ctx; - bool enable; -}; - -struct kirin_plane { - struct drm_plane base; - void *hw_ctx; - u32 ch; -}; - struct ade_data { struct kirin_crtc crtc; struct kirin_plane planes[ADE_CH_NUM]; @@ -78,12 +60,7 @@ struct ade_data { }; /* ade-format info: */ -struct ade_format { - u32 pixel_format; - enum ade_fb_format ade_format; -}; - -static const struct ade_format ade_formats[] = { +static const struct kirin_format ade_formats[] = { /* 16bpp RGB: */ { DRM_FORMAT_RGB565, ADE_RGB_565 }, { DRM_FORMAT_BGR565, ADE_BGR_565 }, @@ -127,7 +104,7 @@ static u32 ade_get_format(u32 pixel_format) for (i = 0; i < ARRAY_SIZE(ade_formats); i++) if (ade_formats[i].pixel_format == pixel_format) - return ade_formats[i].ade_format; + return ade_formats[i].hw_format; /* not found */ DRM_ERROR("Not found pixel format!!fourcc_format= %d\n", diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h index ad027d1..b6626f5 100644 --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h @@ -13,6 +13,30 @@ #define MAX_CRTC 2 +#define to_kirin_crtc(crtc) \ + container_of(crtc, struct kirin_crtc, base) + +#define to_kirin_plane(plane) \ + container_of(plane, struct kirin_plane, base) + +/* kirin-format translate table */ +struct kirin_format { + u32 pixel_format; + u32 hw_format; +}; + +struct kirin_crtc { + struct drm_crtc base; + void *hw_ctx; + bool enable; +}; + +struct kirin_plane { + struct drm_plane base; + void *hw_ctx; + u32 ch; +}; + /* display controller init/cleanup ops */ struct kirin_dc_ops { int (*init)(struct platform_device *pdev);