<div dir="ltr">Hi Sachin,<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Mar 19, 2014 at 10:51 PM, Sachin Kamat <span dir="ltr"><<a href="mailto:sachin.kamat@linaro.org" target="_blank">sachin.kamat@linaro.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Ajay,<br>
<div><div class="h5"><br>
On 19 March 2014 19:52, Ajay Kumar <<a href="mailto:ajaykumar.rs@samsung.com">ajaykumar.rs@samsung.com</a>> wrote:<br>
> Add post processor ops for MDNIE and their support functions.<br>
> Expose an interface for the FIMD to register MDNIE PP.<br>
><br>
> Signed-off-by: Ajay Kumar <<a href="mailto:ajaykumar.rs@samsung.com">ajaykumar.rs@samsung.com</a>><br>
> Signed-off-by: Shirish S <<a href="mailto:s.shirish@samsung.com">s.shirish@samsung.com</a>><br>
> Signed-off-by: Rahul Sharma <<a href="mailto:rahul.sharma@samsung.com">rahul.sharma@samsung.com</a>><br>
> ---<br>
>  drivers/gpu/drm/exynos/Makefile            |   2 +-<br>
>  drivers/gpu/drm/exynos/exynos_mdnie.c      | 707 +++++++++++++++++++++++++++++<br>
>  drivers/gpu/drm/exynos/exynos_mdnie_regs.h | 154 +++++++<br>
>  3 files changed, 862 insertions(+), 1 deletion(-)<br>
>  create mode 100644 drivers/gpu/drm/exynos/exynos_mdnie.c<br>
>  create mode 100644 drivers/gpu/drm/exynos/exynos_mdnie_regs.h<br>
><br>
> diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile<br>
> index 02dde22..653eab5 100644<br>
> --- a/drivers/gpu/drm/exynos/Makefile<br>
> +++ b/drivers/gpu/drm/exynos/Makefile<br>
> @@ -10,7 +10,7 @@ exynosdrm-y := exynos_drm_drv.o exynos_drm_encoder.o \<br>
><br>
>  exynosdrm-$(CONFIG_DRM_EXYNOS_IOMMU) += exynos_drm_iommu.o<br>
>  exynosdrm-$(CONFIG_DRM_EXYNOS_DMABUF) += exynos_drm_dmabuf.o<br>
> -exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)    += exynos_drm_fimd.o<br>
> +exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)    += exynos_drm_fimd.o exynos_mdnie.o<br>
>  exynosdrm-$(CONFIG_DRM_EXYNOS_DSI)     += exynos_drm_dsi.o<br>
>  exynosdrm-$(CONFIG_DRM_EXYNOS_DP)      += exynos_dp_core.o exynos_dp_reg.o<br>
>  exynosdrm-$(CONFIG_DRM_EXYNOS_HDMI)    += exynos_hdmi.o exynos_mixer.o<br>
> diff --git a/drivers/gpu/drm/exynos/exynos_mdnie.c b/drivers/gpu/drm/exynos/exynos_mdnie.c<br>
> new file mode 100644<br>
> index 0000000..a043853<br>
> --- /dev/null<br>
> +++ b/drivers/gpu/drm/exynos/exynos_mdnie.c<br>
> @@ -0,0 +1,707 @@<br>
> +/* drivers/gpu/drm/exynos/exynos_mdnie.c<br>
> + *<br>
> + * Samsung mDNIe driver<br>
> + *<br>
> + * Copyright (C) 2014 Samsung Electronics Co., Ltd.<br>
> + *<br>
> + * This program is free software; you can redistribute it and/or modify it<br>
> + * under the terms of the GNU General Public License as published by the<br>
> + * Free Software Foundation; either version 2 of the License, or (at your<br>
> + * option) any later version.<br>
> +*/<br>
> +<br>
> +#include <linux/clk.h><br>
> +#include <linux/errno.h><br>
> +#include <linux/of_device.h><br>
> +<br>
> +#include <video/samsung_fimd.h><br>
> +<br>
> +#include <drm/drmP.h><br>
> +<br>
> +#include "exynos_drm_drv.h"<br>
> +#include "exynos_fimd_pp.h"<br>
> +#include "exynos_mdnie_regs.h"<br>
> +<br>
> +#define GAMMA_RAMP_LENGTH      17<br>
> +#define COLOR_COMPONENTS       3<br>
> +<br>
> +#define MDNIE_ON       1<br>
> +#define MDNIE_OFF      0<br>
> +<br>
> +#define PARAM_IN_RANGE(r, p, l, u)                                     \<br>
> +       do {                                                    \<br>
> +               r = ((p >= l) && (p <= u)) ? 0 : 1;             \<br>
> +               if (r) \<br>
> +                       DRM_ERROR("Wrong param: %s:%llu\n", #p, (u64)p); \<br>
> +       } while (0)<br>
> +<br>
<br>
</div></div>[snip]<br>
<div class=""><br>
> +static int mdnie_set_color_saturation_params(<br>
> +               struct mdnie_context *mdnie,<br>
> +               const struct drm_mode_color_saturation *params)<br>
> +{<br>
> +       int ret;<br>
> +<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_red, HG_MIN, HG_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_green, HG_MIN, HG_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_blue, HG_MIN, HG_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_cyan, HG_MIN, HG_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_magenta, HG_MIN, HG_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_yellow, HG_MIN, HG_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->hue_gain_overall, HG_MIN, HG_MAX);<br>
> +<br>
> +       if (ret)<br>
> +               return -EINVAL;<br>
<br>
</div>This would be applicable only for the last macro call as ret would get<br>
overwritten after<br>
each call.<br>
<div><div class="h5"><br></div></div></blockquote><div>Nice catch. I will fix it. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
> +       memcpy(&mdnie->params.cs_params, params, sizeof(*params));<br>
> +<br>
> +       return 0;<br>
> +}<br>
> +<br>
> +static int mdnie_set_color_reproduction_params(<br>
> +               struct mdnie_context *mdnie,<br>
> +               const struct drm_mode_color_reproduction *params)<br>
> +{<br>
> +       int ret;<br>
> +<br>
> +       PARAM_IN_RANGE(ret, params->red_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->red_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->red_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->green_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->green_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->green_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->blue_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->blue_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->blue_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->cyan_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->cyan_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->cyan_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->magenta_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->magenta_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->magenta_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->yellow_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->yellow_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->yellow_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->white_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->white_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->white_rgb[2], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->black_rgb[0], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->black_rgb[1], CC_MIN, CC_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->black_rgb[2], CC_MIN, CC_MAX);<br>
> +<br>
> +       if (ret)<br>
> +               return -EINVAL;<br>
><br>
</div></div>ditto<br>
<div class=""><br></div></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> +       memcpy(&mdnie->params.cr_params, params, sizeof(*params));<br>
> +       return 0;<br>
> +}<br>
> +<br>
> +static int mdnie_set_edge_enhancement_params(<br>
> +               struct mdnie_context *mdnie,<br>
> +               const struct drm_mode_edge_enhancement *params)<br>
> +{<br>
> +       int ret;<br>
> +<br>
> +       PARAM_IN_RANGE(ret, params->edge_th, TH_MIN, TH_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->background_th, TH_MIN, TH_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->pg_edge, GAIN_MIN, GAIN_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->pg_flat, GAIN_MIN, GAIN_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->pg_background, GAIN_MIN, GAIN_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->ng_edge, GAIN_MIN, GAIN_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->ng_flat, GAIN_MIN, GAIN_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->ng_background, GAIN_MIN, GAIN_MAX);<br>
<br>
</div>ditto<br></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class=""><br>
> +       if (ret)<br>
> +               return -EINVAL;<br>
> +<br>
> +       memcpy(&mdnie->params.de_params, params, sizeof(*params));<br>
> +       return 0;<br>
> +}<br>
> +<br>
> +static int mdnie_set_window_params(<br>
> +               struct mdnie_context *mdnie,<br>
> +               const struct drm_exynos_mdnie_window *params)<br>
> +{<br>
> +       int ret;<br>
> +<br>
> +       PARAM_IN_RANGE(ret, params->win_x, X_MIN, X_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->win_y, Y_MIN, Y_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->win_w, params->win_x, X_MAX);<br>
> +       PARAM_IN_RANGE(ret, params->win_h, params->win_y, Y_MAX);<br>
<br>
</div>ditto and all other places of similar usage.<br>
<br></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<br>
<br>
<br>
<br>
[snip]<br>
<div class=""><br>
> +int exynos_mdnie_init(struct device *dev, struct exynos_fimd_pp **pp)<br>
> +{<br>
> +       struct device_node *np = dev->of_node;<br>
> +       struct device_node *mdnie_np;<br>
> +       struct mdnie_context *mdnie = NULL;<br>
> +       u32 disp1blk_phyaddr;<br>
> +       int ret = 0;<br>
> +       u32 buf[2];<br>
> +<br>
> +       mdnie_np = of_parse_phandle(np, "samsung,mdnie", 0);<br>
> +       if (!mdnie_np) {<br>
> +               DRM_INFO("No mdnie node present, "<br>
> +                               "MDNIE feature will be disabled\n");<br>
> +               ret = -EINVAL;<br>
> +               goto err0;<br>
<br>
</div>return directly from here.<br>
<div class=""><br></div></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> +       }<br>
> +<br>
> +       if (of_property_read_u32_array(mdnie_np, "reg", buf, 2)) {<br>
> +               DRM_ERROR("failed to get base address for MDNIE\n");<br>
> +               ret = -ENOMEM;<br>
> +               goto err0;<br>
<br>
</div>ditto<br></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class=""> <br></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> +       }<br>
> +<br>
> +       mdnie = kzalloc(sizeof(struct mdnie_context), GFP_KERNEL);<br>
<br>
</div>nit: use sizeof(*mdnie)<br>
<div class=""><br></div></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
> +       if (!mdnie) {<br>
> +               DRM_ERROR("failed to allocate mdnie\n");<br>
<br>
</div>This message is not needed as kzalloc prints OOM message upon failure.<br></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class=""><br>
> +               ret = -ENOMEM;<br>
> +               goto err0;<br>
> +       }<br>
<br>
</div>return directly. </blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class=""><br>
> +<br>
> +       mdnie->exynos_mdnie_base = ioremap(buf[0], buf[1]);<br>
> +       if (!mdnie->exynos_mdnie_base) {<br>
> +               DRM_ERROR("failed to ioremap mdnie device\n");<br>
> +               ret = -ENOMEM;<br>
> +               goto err1;<br>
> +       }<br>
> +<br>
> +       if (dt_parse_disp1blk_cfg(dev, &disp1blk_phyaddr)) {<br>
> +               DRM_ERROR("failed to get disp1blk property.\n");<br>
> +               ret = -ENODEV;<br>
> +               goto err1;<br>
> +       }<br>
> +<br>
> +       if (exynos_iomap_disp1blk(mdnie, disp1blk_phyaddr)) {<br>
> +               DRM_ERROR("failed to iopmap disp1blk sysreg.\n");<br>
<br>
</div>s/iopmap/iomap<br>
<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div>Ok. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="HOEnZb"><font color="#888888">
<br>
--<br>
With warm regards,<br>
Sachin<br></font></span></blockquote><div> </div></div>I will address your comments in next patchset.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks,</div><div class="gmail_extra">Ajay</div></div>