Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH 02/20] drm: Remove users of drm_format_(horz|vert)_chroma_subsampling | From | Philipp Zabel <> | Date | Wed, 17 Apr 2019 15:32:17 +0200 |
| |
On Wed, 2019-04-17 at 09:54 +0200, Maxime Ripard wrote: > drm_format_horz_chroma_subsampling and drm_format_vert_chroma_subsampling > are basically a lookup in the drm_format_info table plus an access to the > hsub and vsub fields of the appropriate entry. > > Most drivers are using this function while having access to the entry > already, which means that we will perform an unnecessary lookup. Removing > the call to these functions is therefore more efficient. > > Some drivers will not have access to that entry in the function, but in > this case the overhead is minimal (we just have to call drm_format_info() > to perform the lookup) and we can even avoid multiple, inefficient lookups > in some places that need multiple fields from the drm_format_info > structure. > > This is amplified by the fact that most of the time the callers will have > to retrieve both the vsub and hsub fields, meaning that they would perform > twice the lookup. > > Reviewed-by: Emil Velikov <emil.velikov@collabora.com> > Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Just one small suggestion:
[...] > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 20a9c296d027..345d5cb5e956 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -317,21 +317,18 @@ static void scl_vop_cal_scl_fac(struct vop *vop, const struct vop_win_data *win, > uint32_t src_w, uint32_t src_h, uint32_t dst_w, > uint32_t dst_h, uint32_t pixel_format) > { > + const struct drm_format_info *info = drm_format_info(pixel_format);
The only call site in vop_plane_atomic_update passes fb->format->format as pixel_format parameter, so this could be simplified even further by replacing the uint32_t pixelformat argument with a const struct drm_format_info *info, and passing fb->format directly instead.
regards Philipp
| ![\](/images/icornerr.gif) |