Message ID | 1504850560-27950-1-git-send-email-hoegeun.kwon@samsung.com |
---|---|
Headers | show |
Series | Exynos-gsc: Support the hardware rotation limits | expand |
On 08/09/17 07:02, Hoegeun Kwon wrote: > Exynos 5250 and 5420 have different hardware rotation limits. However, > currently it uses only one compatible - "exynos5-gsc". Since we have > to distinguish between these two, we add different compatible. > > Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > --- > arch/arm/boot/dts/exynos5250.dtsi | 8 ++++---- > arch/arm/boot/dts/exynos5420.dtsi | 4 ++-- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi > index 8dbeb87..bf08101 100644 > --- a/arch/arm/boot/dts/exynos5250.dtsi > +++ b/arch/arm/boot/dts/exynos5250.dtsi > @@ -637,7 +637,7 @@ > }; > > gsc_0: gsc@13e00000 { > - compatible = "samsung,exynos5-gsc"; > + compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc"; These should be the other way round - the most specific compatible should come first, then the more general fallback afterwards. (and similarly in all cases below) Robin. > reg = <0x13e00000 0x1000>; > interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>; > power-domains = <&pd_gsc>; > @@ -647,7 +647,7 @@ > }; > > gsc_1: gsc@13e10000 { > - compatible = "samsung,exynos5-gsc"; > + compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc"; > reg = <0x13e10000 0x1000>; > interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>; > power-domains = <&pd_gsc>; > @@ -657,7 +657,7 @@ > }; > > gsc_2: gsc@13e20000 { > - compatible = "samsung,exynos5-gsc"; > + compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc"; > reg = <0x13e20000 0x1000>; > interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>; > power-domains = <&pd_gsc>; > @@ -667,7 +667,7 @@ > }; > > gsc_3: gsc@13e30000 { > - compatible = "samsung,exynos5-gsc"; > + compatible = "samsung,exynos5-gsc", "samsung,exynos5250-gsc"; > reg = <0x13e30000 0x1000>; > interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>; > power-domains = <&pd_gsc>; > diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi > index 02d2f89..86afe77 100644 > --- a/arch/arm/boot/dts/exynos5420.dtsi > +++ b/arch/arm/boot/dts/exynos5420.dtsi > @@ -658,7 +658,7 @@ > }; > > gsc_0: video-scaler@13e00000 { > - compatible = "samsung,exynos5-gsc"; > + compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc"; > reg = <0x13e00000 0x1000>; > interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&clock CLK_GSCL0>; > @@ -668,7 +668,7 @@ > }; > > gsc_1: video-scaler@13e10000 { > - compatible = "samsung,exynos5-gsc"; > + compatible = "samsung,exynos5-gsc", "samsung,exynos5420-gsc"; > reg = <0x13e10000 0x1000>; > interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&clock CLK_GSCL1>; > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/09/17 07:02, Hoegeun Kwon wrote: > The gscaler has hardware rotation limits that need to be hardcoded > into driver. Distinguish them and add them to the property list. > > The hardware rotation limits are related to the cropped source size. > When swap occurs, use rot_max size instead of crop_max size. > > Also the scaling limits are related to pos size, use pos size to check > the limits. > > Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > --- > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 93 +++++++++++++++++++++++---------- > include/uapi/drm/exynos_drm.h | 2 + > 2 files changed, 66 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c > index 0506b2b..a4fb347 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c > @@ -17,6 +17,7 @@ > #include <linux/pm_runtime.h> > #include <linux/mfd/syscon.h> > #include <linux/regmap.h> > +#include <linux/of_device.h> > > #include <drm/drmP.h> > #include <drm/exynos_drm.h> > @@ -150,6 +151,15 @@ struct gsc_context { > bool suspended; > }; > > +/* > + * struct gsc_driverdata - per device type driver data for init time. > + * > + * @rot_max: rotation max resolution. > + */ > +struct gsc_driverdata { > + struct drm_exynos_sz rot_max; > +}; > + > /* 8-tap Filter Coefficient */ > static const int h_coef_8t[GSC_COEF_RATIO][GSC_COEF_ATTR][GSC_COEF_H_8T] = { > { /* Ratio <= 65536 (~8:8) */ > @@ -1401,6 +1411,23 @@ static int gsc_ippdrv_check_property(struct device *dev, > bool swap; > int i; > > + config = &property->config[EXYNOS_DRM_OPS_DST]; > + > + /* check for degree */ > + switch (config->degree) { > + case EXYNOS_DRM_DEGREE_90: > + case EXYNOS_DRM_DEGREE_270: > + swap = true; > + break; > + case EXYNOS_DRM_DEGREE_0: > + case EXYNOS_DRM_DEGREE_180: > + swap = false; > + break; > + default: > + DRM_ERROR("invalid degree.\n"); > + goto err_property; > + } > + > for_each_ipp_ops(i) { > if ((i == EXYNOS_DRM_OPS_SRC) && > (property->cmd == IPP_CMD_WB)) > @@ -1416,21 +1443,6 @@ static int gsc_ippdrv_check_property(struct device *dev, > goto err_property; > } > > - /* check for degree */ > - switch (config->degree) { > - case EXYNOS_DRM_DEGREE_90: > - case EXYNOS_DRM_DEGREE_270: > - swap = true; > - break; > - case EXYNOS_DRM_DEGREE_0: > - case EXYNOS_DRM_DEGREE_180: > - swap = false; > - break; > - default: > - DRM_ERROR("invalid degree.\n"); > - goto err_property; > - } > - > /* check for buffer bound */ > if ((pos->x + pos->w > sz->hsize) || > (pos->y + pos->h > sz->vsize)) { > @@ -1438,21 +1450,27 @@ static int gsc_ippdrv_check_property(struct device *dev, > goto err_property; > } > > + /* > + * The rotation hardware limits are related to the cropped > + * source size. So use rot_max size to check the limits when > + * swap happens. And also the scaling limits are related to pos > + * size, use pos size to check the limits. > + */ > /* check for crop */ > if ((i == EXYNOS_DRM_OPS_SRC) && (pp->crop)) { > if (swap) { > if ((pos->h < pp->crop_min.hsize) || > - (sz->vsize > pp->crop_max.hsize) || > + (pos->h > pp->rot_max.hsize) || > (pos->w < pp->crop_min.vsize) || > - (sz->hsize > pp->crop_max.vsize)) { > + (pos->w > pp->rot_max.vsize)) { > DRM_ERROR("out of crop size.\n"); > goto err_property; > } > } else { > if ((pos->w < pp->crop_min.hsize) || > - (sz->hsize > pp->crop_max.hsize) || > + (pos->w > pp->crop_max.hsize) || > (pos->h < pp->crop_min.vsize) || > - (sz->vsize > pp->crop_max.vsize)) { > + (pos->h > pp->crop_max.vsize)) { > DRM_ERROR("out of crop size.\n"); > goto err_property; > } > @@ -1463,17 +1481,17 @@ static int gsc_ippdrv_check_property(struct device *dev, > if ((i == EXYNOS_DRM_OPS_DST) && (pp->scale)) { > if (swap) { > if ((pos->h < pp->scale_min.hsize) || > - (sz->vsize > pp->scale_max.hsize) || > + (pos->h > pp->scale_max.hsize) || > (pos->w < pp->scale_min.vsize) || > - (sz->hsize > pp->scale_max.vsize)) { > + (pos->w > pp->scale_max.vsize)) { > DRM_ERROR("out of scale size.\n"); > goto err_property; > } > } else { > if ((pos->w < pp->scale_min.hsize) || > - (sz->hsize > pp->scale_max.hsize) || > + (pos->w > pp->scale_max.hsize) || > (pos->h < pp->scale_min.vsize) || > - (sz->vsize > pp->scale_max.vsize)) { > + (pos->h > pp->scale_max.vsize)) { > DRM_ERROR("out of scale size.\n"); > goto err_property; > } > @@ -1657,12 +1675,34 @@ static void gsc_ippdrv_stop(struct device *dev, enum drm_exynos_ipp_cmd cmd) > gsc_write(cfg, GSC_ENABLE); > } > > +static struct gsc_driverdata gsc_exynos5250_drvdata = { > + .rot_max = { 2048, 2048 }, > +}; > + > +static struct gsc_driverdata gsc_exynos5420_drvdata = { > + .rot_max = { 2016, 2016 }, > +}; > + > +static const struct of_device_id exynos_drm_gsc_of_match[] = { > + { > + .compatible = "samsung,exynos5250-gsc", > + .data = &gsc_exynos5250_drvdata, > + }, > + { > + .compatible = "samsung,exynos5420-gsc", > + .data = &gsc_exynos5420_drvdata, > + }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, exynos_drm_gsc_of_match); > + > static int gsc_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct gsc_context *ctx; > struct resource *res; > struct exynos_drm_ippdrv *ippdrv; > + const struct gsc_driverdata *drv_data = of_device_get_match_data(dev); > int ret; > > ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > @@ -1722,6 +1762,7 @@ static int gsc_probe(struct platform_device *pdev) > dev_err(dev, "failed to init property list.\n"); > return ret; > } > + ctx->ippdrv.prop_list.rot_max = drv_data->rot_max; > > DRM_DEBUG_KMS("id[%d]ippdrv[%pK]\n", ctx->id, ippdrv); > > @@ -1784,12 +1825,6 @@ static int __maybe_unused gsc_runtime_resume(struct device *dev) > SET_RUNTIME_PM_OPS(gsc_runtime_suspend, gsc_runtime_resume, NULL) > }; > > -static const struct of_device_id exynos_drm_gsc_of_match[] = { > - { .compatible = "samsung,exynos5-gsc" }, > - { }, > -}; > -MODULE_DEVICE_TABLE(of, exynos_drm_gsc_of_match); As Krzysztof noted, this breaks compatibility with existing DTBs, and not everyone wants to update their firmware in lock-step with their kernel. That said, as far as I can see there's little need to remove it anyway - as long as there is some set of lowest-common-denominator limits that will work on any hardware variant, there doesn't seem to be much harm in keeping a generic fallback around indefinitely. Robin. > - > struct platform_driver gsc_driver = { > .probe = gsc_probe, > .remove = gsc_remove, > diff --git a/include/uapi/drm/exynos_drm.h b/include/uapi/drm/exynos_drm.h > index cb3e9f9..d5d5518 100644 > --- a/include/uapi/drm/exynos_drm.h > +++ b/include/uapi/drm/exynos_drm.h > @@ -192,6 +192,7 @@ enum drm_exynos_planer { > * @crop_max: crop max resolution. > * @scale_min: scale min resolution. > * @scale_max: scale max resolution. > + * @rot_max: rotation max resolution. > */ > struct drm_exynos_ipp_prop_list { > __u32 version; > @@ -210,6 +211,7 @@ struct drm_exynos_ipp_prop_list { > struct drm_exynos_sz crop_max; > struct drm_exynos_sz scale_min; > struct drm_exynos_sz scale_max; > + struct drm_exynos_sz rot_max; > }; > > /** > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/08/2017 08:02 AM, Hoegeun Kwon wrote: > The hardware rotation limits of gsc depends on SOC (Exynos > 5250/5420/5433). Distinguish them and add them to the driver data. > > Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> > --- > drivers/media/platform/exynos-gsc/gsc-core.c | 96 ++++++++++++++++++++++++---- > 1 file changed, 83 insertions(+), 13 deletions(-) > > diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c > index 4380150..8f8636e 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-core.c > +++ b/drivers/media/platform/exynos-gsc/gsc-core.c > @@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv) > return IRQ_HANDLED; > } > > -static struct gsc_pix_max gsc_v_100_max = { > +static struct gsc_pix_max gsc_v_5250_max = { > + .org_scaler_bypass_w = 8192, > + .org_scaler_bypass_h = 8192, > + .org_scaler_input_w = 4800, > + .org_scaler_input_h = 3344, > + .real_rot_dis_w = 4800, > + .real_rot_dis_h = 3344, > + .real_rot_en_w = 2016, > + .real_rot_en_h = 2016, > + .target_rot_dis_w = 4800, > + .target_rot_dis_h = 3344, > + .target_rot_en_w = 2016, > + .target_rot_en_h = 2016, > +}; > + > +static struct gsc_pix_max gsc_v_5420_max = { > + .org_scaler_bypass_w = 8192, > + .org_scaler_bypass_h = 8192, > + .org_scaler_input_w = 4800, > + .org_scaler_input_h = 3344, > + .real_rot_dis_w = 4800, > + .real_rot_dis_h = 3344, > + .real_rot_en_w = 2048, > + .real_rot_en_h = 2048, > + .target_rot_dis_w = 4800, > + .target_rot_dis_h = 3344, > + .target_rot_en_w = 2016, > + .target_rot_en_h = 2016, > +}; > + > +static struct gsc_pix_max gsc_v_5433_max = { > .org_scaler_bypass_w = 8192, > .org_scaler_bypass_h = 8192, > .org_scaler_input_w = 4800, > @@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv) > .target_h = 2, /* yuv420 : 2, others : 1 */ > }; > > -static struct gsc_variant gsc_v_100_variant = { > - .pix_max = &gsc_v_100_max, > +static struct gsc_variant gsc_v_5250_variant = { > + .pix_max = &gsc_v_5250_max, > .pix_min = &gsc_v_100_min, > .pix_align = &gsc_v_100_align, > .in_buf_cnt = 32, > @@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv) > .local_sc_down = 2, > }; > > -static struct gsc_driverdata gsc_v_100_drvdata = { > +static struct gsc_variant gsc_v_5420_variant = { > + .pix_max = &gsc_v_5420_max, > + .pix_min = &gsc_v_100_min, > + .pix_align = &gsc_v_100_align, > + .in_buf_cnt = 32, > + .out_buf_cnt = 32, > + .sc_up_max = 8, > + .sc_down_max = 16, > + .poly_sc_down_max = 4, > + .pre_sc_down_max = 4, > + .local_sc_down = 2, > +}; > + > +static struct gsc_variant gsc_v_5433_variant = { > + .pix_max = &gsc_v_5433_max, > + .pix_min = &gsc_v_100_min, > + .pix_align = &gsc_v_100_align, > + .in_buf_cnt = 32, > + .out_buf_cnt = 32, > + .sc_up_max = 8, > + .sc_down_max = 16, > + .poly_sc_down_max = 4, > + .pre_sc_down_max = 4, > + .local_sc_down = 2, > +}; > + > +static struct gsc_driverdata gsc_v_5250_drvdata = { > .variant = { > - [0] = &gsc_v_100_variant, > - [1] = &gsc_v_100_variant, > - [2] = &gsc_v_100_variant, > - [3] = &gsc_v_100_variant, > + [0] = &gsc_v_5250_variant, > + [1] = &gsc_v_5250_variant, > + [2] = &gsc_v_5250_variant, > + [3] = &gsc_v_5250_variant, > + }, > + .num_entities = 4, > + .clk_names = { "gscl" }, > + .num_clocks = 1, > +}; > + > +static struct gsc_driverdata gsc_v_5420_drvdata = { > + .variant = { > + [0] = &gsc_v_5420_variant, > + [1] = &gsc_v_5420_variant, > }, > .num_entities = 4, > .clk_names = { "gscl" }, > @@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv) > > static struct gsc_driverdata gsc_5433_drvdata = { > .variant = { > - [0] = &gsc_v_100_variant, > - [1] = &gsc_v_100_variant, > - [2] = &gsc_v_100_variant, > + [0] = &gsc_v_5433_variant, > + [1] = &gsc_v_5433_variant, > + [2] = &gsc_v_5433_variant, > }, > .num_entities = 3, > .clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" }, > @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, void *priv) > > static const struct of_device_id exynos_gsc_match[] = { > { > - .compatible = "samsung,exynos5-gsc", > - .data = &gsc_v_100_drvdata, Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant data, so that it can work with "samsung,exynos5-gsc" compatible in DT on both exynos5250 and exynos5420 SoCs? > + .compatible = "samsung,exynos5250-gsc", > + .data = &gsc_v_5250_drvdata, > + }, > + { > + .compatible = "samsung,exynos5420-gsc", > + .data = &gsc_v_5420_drvdata, > },
Hi Sylwester, On 09/11/2017 06:35 PM, Sylwester Nawrocki wrote: > On 09/08/2017 08:02 AM, Hoegeun Kwon wrote: >> The hardware rotation limits of gsc depends on SOC (Exynos >> 5250/5420/5433). Distinguish them and add them to the driver data. >> >> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com> >> --- >> drivers/media/platform/exynos-gsc/gsc-core.c | 96 >> ++++++++++++++++++++++++---- >> 1 file changed, 83 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c >> b/drivers/media/platform/exynos-gsc/gsc-core.c >> index 4380150..8f8636e 100644 >> --- a/drivers/media/platform/exynos-gsc/gsc-core.c >> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c >> @@ -943,7 +943,37 @@ static irqreturn_t gsc_irq_handler(int irq, void >> *priv) >> return IRQ_HANDLED; >> } >> -static struct gsc_pix_max gsc_v_100_max = { >> +static struct gsc_pix_max gsc_v_5250_max = { >> + .org_scaler_bypass_w = 8192, >> + .org_scaler_bypass_h = 8192, >> + .org_scaler_input_w = 4800, >> + .org_scaler_input_h = 3344, >> + .real_rot_dis_w = 4800, >> + .real_rot_dis_h = 3344, >> + .real_rot_en_w = 2016, >> + .real_rot_en_h = 2016, >> + .target_rot_dis_w = 4800, >> + .target_rot_dis_h = 3344, >> + .target_rot_en_w = 2016, >> + .target_rot_en_h = 2016, >> +}; >> + >> +static struct gsc_pix_max gsc_v_5420_max = { >> + .org_scaler_bypass_w = 8192, >> + .org_scaler_bypass_h = 8192, >> + .org_scaler_input_w = 4800, >> + .org_scaler_input_h = 3344, >> + .real_rot_dis_w = 4800, >> + .real_rot_dis_h = 3344, >> + .real_rot_en_w = 2048, >> + .real_rot_en_h = 2048, >> + .target_rot_dis_w = 4800, >> + .target_rot_dis_h = 3344, >> + .target_rot_en_w = 2016, >> + .target_rot_en_h = 2016, >> +}; >> + >> +static struct gsc_pix_max gsc_v_5433_max = { >> .org_scaler_bypass_w = 8192, >> .org_scaler_bypass_h = 8192, >> .org_scaler_input_w = 4800, >> @@ -979,8 +1009,8 @@ static irqreturn_t gsc_irq_handler(int irq, void >> *priv) >> .target_h = 2, /* yuv420 : 2, others : 1 */ >> }; >> -static struct gsc_variant gsc_v_100_variant = { >> - .pix_max = &gsc_v_100_max, >> +static struct gsc_variant gsc_v_5250_variant = { >> + .pix_max = &gsc_v_5250_max, >> .pix_min = &gsc_v_100_min, >> .pix_align = &gsc_v_100_align, >> .in_buf_cnt = 32, >> @@ -992,12 +1022,48 @@ static irqreturn_t gsc_irq_handler(int irq, >> void *priv) >> .local_sc_down = 2, >> }; >> -static struct gsc_driverdata gsc_v_100_drvdata = { >> +static struct gsc_variant gsc_v_5420_variant = { >> + .pix_max = &gsc_v_5420_max, >> + .pix_min = &gsc_v_100_min, >> + .pix_align = &gsc_v_100_align, >> + .in_buf_cnt = 32, >> + .out_buf_cnt = 32, >> + .sc_up_max = 8, >> + .sc_down_max = 16, >> + .poly_sc_down_max = 4, >> + .pre_sc_down_max = 4, >> + .local_sc_down = 2, >> +}; >> + >> +static struct gsc_variant gsc_v_5433_variant = { >> + .pix_max = &gsc_v_5433_max, >> + .pix_min = &gsc_v_100_min, >> + .pix_align = &gsc_v_100_align, >> + .in_buf_cnt = 32, >> + .out_buf_cnt = 32, >> + .sc_up_max = 8, >> + .sc_down_max = 16, >> + .poly_sc_down_max = 4, >> + .pre_sc_down_max = 4, >> + .local_sc_down = 2, >> +}; >> + >> +static struct gsc_driverdata gsc_v_5250_drvdata = { >> .variant = { >> - [0] = &gsc_v_100_variant, >> - [1] = &gsc_v_100_variant, >> - [2] = &gsc_v_100_variant, >> - [3] = &gsc_v_100_variant, >> + [0] = &gsc_v_5250_variant, >> + [1] = &gsc_v_5250_variant, >> + [2] = &gsc_v_5250_variant, >> + [3] = &gsc_v_5250_variant, >> + }, >> + .num_entities = 4, >> + .clk_names = { "gscl" }, >> + .num_clocks = 1, >> +}; >> + >> +static struct gsc_driverdata gsc_v_5420_drvdata = { >> + .variant = { >> + [0] = &gsc_v_5420_variant, >> + [1] = &gsc_v_5420_variant, >> }, >> .num_entities = 4, >> .clk_names = { "gscl" }, >> @@ -1006,9 +1072,9 @@ static irqreturn_t gsc_irq_handler(int irq, >> void *priv) >> static struct gsc_driverdata gsc_5433_drvdata = { >> .variant = { >> - [0] = &gsc_v_100_variant, >> - [1] = &gsc_v_100_variant, >> - [2] = &gsc_v_100_variant, >> + [0] = &gsc_v_5433_variant, >> + [1] = &gsc_v_5433_variant, >> + [2] = &gsc_v_5433_variant, >> }, >> .num_entities = 3, >> .clk_names = { "pclk", "aclk", "aclk_xiu", "aclk_gsclbend" }, >> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, >> void *priv) >> static const struct of_device_id exynos_gsc_match[] = { >> { >> - .compatible = "samsung,exynos5-gsc", >> - .data = &gsc_v_100_drvdata, > > Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant > data, so that it can work with "samsung,exynos5-gsc" compatible in DT > on both exynos5250 and exynos5420 SoCs? > Thank you for your question. Exynos 5250 and 5420 have different hardware rotation limits. Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h = 2048'. So my opinion they must have different compatible. Best regards, Hoegeun >> + .compatible = "samsung,exynos5250-gsc", >> + .data = &gsc_v_5250_drvdata, >> + }, >> + { >> + .compatible = "samsung,exynos5420-gsc", >> + .data = &gsc_v_5420_drvdata, >> }, > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hoegeun, On 09/13/2017 04:33 AM, Hoegeun Kwon wrote: >>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, >>> void *priv) >>> static const struct of_device_id exynos_gsc_match[] = { >>> { >>> - .compatible = "samsung,exynos5-gsc", >>> - .data = &gsc_v_100_drvdata, >> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant >> data, so that it can work with "samsung,exynos5-gsc" compatible in DT >> on both exynos5250 and exynos5420 SoCs? >> > Thank you for your question. > > Exynos 5250 and 5420 have different hardware rotation limits. > Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h = > 2048'. > > So my opinion they must have different compatible. I think there is some misunderstanding, mu suggestion was to keep the "samsung,exynos5-gsc" compatible entry in addition to the new introduced ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in order to make your changes possibly backward compatible, in theory the updated driver should still work without changes in dts.
On 09/13/2017 06:11 PM, Sylwester Nawrocki wrote: > Hi Hoegeun, > > On 09/13/2017 04:33 AM, Hoegeun Kwon wrote: >>>> @@ -1017,8 +1083,12 @@ static irqreturn_t gsc_irq_handler(int irq, >>>> void *priv) >>>> static const struct of_device_id exynos_gsc_match[] = { >>>> { >>>> - .compatible = "samsung,exynos5-gsc", >>>> - .data = &gsc_v_100_drvdata, >>> Can you keep the "samsung,exynos5-gsc" entry with the gsc_v_5250_variant >>> data, so that it can work with "samsung,exynos5-gsc" compatible in DT >>> on both exynos5250 and exynos5420 SoCs? >>> >> Thank you for your question. >> >> Exynos 5250 and 5420 have different hardware rotation limits. >> Exynos 5250 is '.real_rot_en_w/h = 2016' and 5420 is '.real_rot_en_w/h = >> 2048'. >> >> So my opinion they must have different compatible. > I think there is some misunderstanding, mu suggestion was to keep the > "samsung,exynos5-gsc" compatible entry in addition to the new introduced > ones: "samsung,exynos5250-gsc" and "samsung,exynos5420-gsc". That's in > order to make your changes possibly backward compatible, in theory > the updated driver should still work without changes in dts. Thank you again for your explanation. Yes, I understood. I will keep "samsung,exynos5-gsc" compatible, and add Exynos 5250/5420/5433 compatible. Best regards, Hoegeun -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html