| Message ID | 1c4f6b275597dec7d97b5c1d9f749ba27c0610f3.1511797218.git-series.maxime.ripard@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Maxime, Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard napisal(a): > Add support for the A83T display pipeline. > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > 4 files changed, 14 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index > d4259a4f5171..d6b52e5c48c0 100644 > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > @@ -93,6 +93,7 @@ Required properties: > * allwinner,sun6i-a31s-tcon > * allwinner,sun7i-a20-tcon > * allwinner,sun8i-a33-tcon > + * allwinner,sun8i-a83t-tcon-lcd > * allwinner,sun8i-v3s-tcon > - reg: base address and size of memory-mapped region > - interrupts: interrupt associated to this IP > @@ -224,6 +225,7 @@ supported. > > Required properties: > - compatible: value must be one of: > + * allwinner,sun8i-a83t-de2-mixer What will be the name of the second mixer, once support for HDMI is added? Should we start directly with 0 and 1 postfix ? > * allwinner,sun8i-v3s-de2-mixer > - reg: base address and size of the memory-mapped region. > - clocks: phandles to the clocks feeding the mixer > @@ -253,6 +255,7 @@ Required properties: > * allwinner,sun6i-a31s-display-engine > * allwinner,sun7i-a20-display-engine > * allwinner,sun8i-a33-display-engine > + * allwinner,sun8i-a83t-display-engine > * allwinner,sun8i-v3s-display-engine > > - allwinner,pipelines: list of phandle to the display engine > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c > b/drivers/gpu/drm/sun4i/sun4i_drv.c index 75c76cdd82bc..c418be2f22be 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c > @@ -193,6 +193,7 @@ static bool sun4i_drv_node_is_tcon(struct device_node > *node) of_device_is_compatible(node, "allwinner,sun6i-a31s-tcon") || > of_device_is_compatible(node, "allwinner,sun7i-a20-tcon") || > of_device_is_compatible(node, "allwinner,sun8i-a33-tcon") || > + of_device_is_compatible(node, "allwinner,sun8i-a83t-tcon-lcd") || > of_device_is_compatible(node, "allwinner,sun8i-v3s-tcon"); > } > > @@ -353,6 +354,7 @@ static const struct of_device_id sun4i_drv_of_table[] = > { { .compatible = "allwinner,sun6i-a31s-display-engine" }, > { .compatible = "allwinner,sun7i-a20-display-engine" }, > { .compatible = "allwinner,sun8i-a33-display-engine" }, > + { .compatible = "allwinner,sun8i-a83t-display-engine" }, > { .compatible = "allwinner,sun8i-v3s-display-engine" }, > { } > }; > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 46ce6daa0b1a..871df75793a9 > 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -1132,6 +1132,10 @@ static const struct sun4i_tcon_quirks > sun8i_a33_quirks = { .has_lvds_pll = true, > }; > > +static const struct sun4i_tcon_quirks sun8i_a83t_quirks = { > + /* nothing is supported */ > +}; > + > static const struct sun4i_tcon_quirks sun8i_v3s_quirks = { > /* nothing is supported */ > }; > @@ -1143,6 +1147,7 @@ static const struct of_device_id sun4i_tcon_of_table[] > = { { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks > }, { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, > { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, > + { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = > &sun8i_a83t_quirks }, { .compatible = "allwinner,sun8i-v3s-tcon", .data = > &sun8i_v3s_quirks }, { } > }; > diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c > b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 44d5e639ebb2..5a1376965270 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c > @@ -395,6 +395,10 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg > = { > > static const struct of_device_id sun8i_mixer_of_table[] = { > { > + .compatible = "allwinner,sun8i-a83t-de2-mixer", > + .data = &sun8i_v3s_mixer_cfg, > + }, > + { Maybe you want to squash 12 patch since this works only by luck. Best regards, Jernej > .compatible = "allwinner,sun8i-v3s-de2-mixer", > .data = &sun8i_v3s_mixer_cfg, > }, > -- > git-series 0.9.1
On Mon, Nov 27, 2017 at 11:41 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Add support for the A83T display pipeline. > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ Rob mentioned that he'd like to see these multiple one-liners be in a separate patch. > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > 4 files changed, 14 insertions(+) Also not sure why you have two patches with the same subject. ChenYu
Hi, On Tue, Nov 28, 2017 at 12:19:44AM +0800, Chen-Yu Tsai wrote: > On Mon, Nov 27, 2017 at 11:41 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > Add support for the A83T display pipeline. > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ > > Rob mentioned that he'd like to see these multiple one-liners be > in a separate patch. I'll change that. > > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > > 4 files changed, 14 insertions(+) > > Also not sure why you have two patches with the same subject. Yeah, it's a rebase gone wrong... The second one was meant to be folded into this one. Maxime
Hi, On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard napisal(a): > > Add support for the A83T display pipeline. > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > > 4 files changed, 14 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index > > d4259a4f5171..d6b52e5c48c0 100644 > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > @@ -93,6 +93,7 @@ Required properties: > > * allwinner,sun6i-a31s-tcon > > * allwinner,sun7i-a20-tcon > > * allwinner,sun8i-a33-tcon > > + * allwinner,sun8i-a83t-tcon-lcd > > * allwinner,sun8i-v3s-tcon > > - reg: base address and size of memory-mapped region > > - interrupts: interrupt associated to this IP > > @@ -224,6 +225,7 @@ supported. > > > > Required properties: > > - compatible: value must be one of: > > + * allwinner,sun8i-a83t-de2-mixer > > What will be the name of the second mixer, once support for HDMI is added? > Should we start directly with 0 and 1 postfix ? What are the differences exactly without the two mixers? I was hoping to be able to cover them all using properties, indices are usually pretty badly received in compatibles. > > static const struct of_device_id sun8i_mixer_of_table[] = { > > { > > + .compatible = "allwinner,sun8i-a83t-de2-mixer", > > + .data = &sun8i_v3s_mixer_cfg, > > + }, > > + { > > Maybe you want to squash 12 patch since this works only by luck. Yeah, I totally meant to do that :) Thanks! Maxime
在 2017-11-28 17:02，Maxime Ripard 写道： > Hi, > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: >> Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard >> napisal(a): >> > Add support for the A83T display pipeline. >> > >> > Reviewed-by: Chen-Yu Tsai <wens@csie.org> >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> > --- >> > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ >> > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ >> > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ >> > 4 files changed, 14 insertions(+) >> > >> > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt >> > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index >> > d4259a4f5171..d6b52e5c48c0 100644 >> > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt >> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt >> > @@ -93,6 +93,7 @@ Required properties: >> > * allwinner,sun6i-a31s-tcon >> > * allwinner,sun7i-a20-tcon >> > * allwinner,sun8i-a33-tcon >> > + * allwinner,sun8i-a83t-tcon-lcd >> > * allwinner,sun8i-v3s-tcon >> > - reg: base address and size of memory-mapped region >> > - interrupts: interrupt associated to this IP >> > @@ -224,6 +225,7 @@ supported. >> > >> > Required properties: >> > - compatible: value must be one of: >> > + * allwinner,sun8i-a83t-de2-mixer >> >> What will be the name of the second mixer, once support for HDMI is >> added? >> Should we start directly with 0 and 1 postfix ? > > What are the differences exactly without the two mixers? > > I was hoping to be able to cover them all using properties, indices > are usually pretty badly received in compatibles. 1. VEP is only available on the VI channel in mixer0. (VEP may mean Video Enhance Processor) 2. Smart Backlight is only available in mixer0's on SoCs with LCD. 3. Writeback function is only available in mixer0. > >> > static const struct of_device_id sun8i_mixer_of_table[] = { >> > { >> > + .compatible = "allwinner,sun8i-a83t-de2-mixer", >> > + .data = &sun8i_v3s_mixer_cfg, >> > + }, >> > + { >> >> Maybe you want to squash 12 patch since this works only by luck. > > Yeah, I totally meant to do that :) > > Thanks! > Maxime
On Tue, Nov 28, 2017 at 07:50:19PM +0800, Icenowy Zheng wrote: > 在 2017-11-28 17:02，Maxime Ripard 写道： > > Hi, > > > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard > > > napisal(a): > > > > Add support for the A83T display pipeline. > > > > > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > --- > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ > > > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > > > > 4 files changed, 14 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index > > > > d4259a4f5171..d6b52e5c48c0 100644 > > > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > @@ -93,6 +93,7 @@ Required properties: > > > > * allwinner,sun6i-a31s-tcon > > > > * allwinner,sun7i-a20-tcon > > > > * allwinner,sun8i-a33-tcon > > > > + * allwinner,sun8i-a83t-tcon-lcd > > > > * allwinner,sun8i-v3s-tcon > > > > - reg: base address and size of memory-mapped region > > > > - interrupts: interrupt associated to this IP > > > > @@ -224,6 +225,7 @@ supported. > > > > > > > > Required properties: > > > > - compatible: value must be one of: > > > > + * allwinner,sun8i-a83t-de2-mixer > > > > > > What will be the name of the second mixer, once support for HDMI is > > > added? > > > Should we start directly with 0 and 1 postfix ? > > > > What are the differences exactly without the two mixers? > > > > I was hoping to be able to cover them all using properties, indices > > are usually pretty badly received in compatibles. > > 1. VEP is only available on the VI channel in mixer0. (VEP may mean > Video Enhance Processor) > 2. Smart Backlight is only available in mixer0's on SoCs with LCD. > 3. Writeback function is only available in mixer0. Then yeah, we can totally support that using properties. Maxime
Hi, Dne torek, 28. november 2017 ob 10:02:23 CET je Maxime Ripard napisal(a): > Hi, > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard napisal(a): > > > Add support for the A83T display pipeline. > > > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > --- > > > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ > > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > > > 4 files changed, 14 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index > > > d4259a4f5171..d6b52e5c48c0 100644 > > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > @@ -93,6 +93,7 @@ Required properties: > > > * allwinner,sun6i-a31s-tcon > > > * allwinner,sun7i-a20-tcon > > > * allwinner,sun8i-a33-tcon > > > > > > + * allwinner,sun8i-a83t-tcon-lcd > > > > > > * allwinner,sun8i-v3s-tcon > > > > > > - reg: base address and size of memory-mapped region > > > - interrupts: interrupt associated to this IP > > > > > > @@ -224,6 +225,7 @@ supported. > > > > > > Required properties: > > > - compatible: value must be one of: > > > + * allwinner,sun8i-a83t-de2-mixer > > > > What will be the name of the second mixer, once support for HDMI is added? > > Should we start directly with 0 and 1 postfix ? > > What are the differences exactly without the two mixers? Mixer properties: - mixer index (0 or 1), important for determining CCSC base (see my patches) - number of VI planes (usually 1) - number of UI planes (usually 1 or 3) - writeback support (yes/no) - scale line buffer length (2048 or 4096) - smart backligth support (yes/no) channel properties (for both, VI and UI): - scaler support (yes/no, usually yes) - overlay count (seems to be always 4) - VEP support (yes/no) Those are properties found in BSP de_feat.c, so I guess that's enough to make any kind of decision in the code. Usually, but we can't count on that, first mixer has 1 VI and 3 UI planes and second mixer has 1 VI and 1 UI plane. Best regards, Jernej > > I was hoping to be able to cover them all using properties, indices > are usually pretty badly received in compatibles. > > > > static const struct of_device_id sun8i_mixer_of_table[] = { > > > > > > { > > > > > > + .compatible = "allwinner,sun8i-a83t-de2-mixer", > > > + .data = &sun8i_v3s_mixer_cfg, > > > + }, > > > + { > > > > Maybe you want to squash 12 patch since this works only by luck. > > Yeah, I totally meant to do that :) > > Thanks! > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
On Tue, Nov 28, 2017 at 04:48:55PM +0100, Jernej Škrabec wrote: > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard > napisal(a): > > > > Add support for the A83T display pipeline. > > > > > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > --- > > > > > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 +++ > > > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++ > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 +++++ > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 ++++ > > > > 4 files changed, 14 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index > > > > d4259a4f5171..d6b52e5c48c0 100644 > > > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > > @@ -93,6 +93,7 @@ Required properties: > > > > * allwinner,sun6i-a31s-tcon > > > > * allwinner,sun7i-a20-tcon > > > > * allwinner,sun8i-a33-tcon > > > > > > > > + * allwinner,sun8i-a83t-tcon-lcd > > > > > > > > * allwinner,sun8i-v3s-tcon > > > > > > > > - reg: base address and size of memory-mapped region > > > > - interrupts: interrupt associated to this IP > > > > > > > > @@ -224,6 +225,7 @@ supported. > > > > > > > > Required properties: > > > > - compatible: value must be one of: > > > > + * allwinner,sun8i-a83t-de2-mixer > > > > > > What will be the name of the second mixer, once support for HDMI is added? > > > Should we start directly with 0 and 1 postfix ? > > > > What are the differences exactly without the two mixers? > > Mixer properties: > - mixer index (0 or 1), important for determining CCSC base (see my patches) Is that the only thing we need to determine? > - number of VI planes (usually 1) Usually or always? > - number of UI planes (usually 1 or 3) Same question. > - writeback support (yes/no) > - scale line buffer length (2048 or 4096) > - smart backligth support (yes/no) > > channel properties (for both, VI and UI): > - scaler support (yes/no, usually yes) > - overlay count (seems to be always 4) > - VEP support (yes/no) > > Those are properties found in BSP de_feat.c, so I guess that's enough to make > any kind of decision in the code. > > Usually, but we can't count on that, first mixer has 1 VI and 3 UI planes and > second mixer has 1 VI and 1 UI plane. Right. So that would be easy to support using a property as well. The only difference would be the CSC base. Maxime
Hi! Dne torek, 28. november 2017 ob 23:00:14 CET je Maxime Ripard napisal(a): > On Tue, Nov 28, 2017 at 04:48:55PM +0100, Jernej Škrabec wrote: > > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard > > > > napisal(a): > > > > > Add support for the A83T display pipeline. > > > > > > > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > > --- > > > > > > > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 > > > > > +++ > > > > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 > > > > > ++ > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 > > > > > +++++ > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 > > > > > ++++ > > > > > 4 files changed, 14 insertions(+) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > index > > > > > d4259a4f5171..d6b52e5c48c0 100644 > > > > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > > > > @@ -93,6 +93,7 @@ Required properties: > > > > > * allwinner,sun6i-a31s-tcon > > > > > * allwinner,sun7i-a20-tcon > > > > > * allwinner,sun8i-a33-tcon > > > > > > > > > > + * allwinner,sun8i-a83t-tcon-lcd > > > > > > > > > > * allwinner,sun8i-v3s-tcon > > > > > > > > > > - reg: base address and size of memory-mapped region > > > > > - interrupts: interrupt associated to this IP > > > > > > > > > > @@ -224,6 +225,7 @@ supported. > > > > > > > > > > Required properties: > > > > > - compatible: value must be one of: > > > > > + * allwinner,sun8i-a83t-de2-mixer > > > > > > > > What will be the name of the second mixer, once support for HDMI is > > > > added? > > > > Should we start directly with 0 and 1 postfix ? > > > > > > What are the differences exactly without the two mixers? > > > > Mixer properties: > > - mixer index (0 or 1), important for determining CCSC base (see my > > patches) > Is that the only thing we need to determine? For now, mixer index is important only for determining CCSC base in conjuction with VEP capability. Obviously, I can't exclude that there is some other case where that mixer index is needed. Can't we just add reg property for that? > > > - number of VI planes (usually 1) > > Usually or always? V3s mixer has 2 VI channels and others have 1. (Channel is better term, since is used throughout BSP code) > > > - number of UI planes (usually 1 or 3) > > Same question. For now, most SoCs (I didn't check all) have 3 UI channels on first mixer and 1 UI channel on second mixer. Except V3s, which have only one mixer with 1 UI channel. > > > - writeback support (yes/no) > > - scale line buffer length (2048 or 4096) > > - smart backligth support (yes/no) > > > > channel properties (for both, VI and UI): > > - scaler support (yes/no, usually yes) Again, V3s is exception here. Scaler is not supported on UI channel, but other SoCs have scalers on all channels. Disclaimer: I didn't check DE2 capabilities of all SoCs, only few populars. Best regards, Jernej > > - overlay count (seems to be always 4) > > - VEP support (yes/no) > > > > Those are properties found in BSP de_feat.c, so I guess that's enough to > > make any kind of decision in the code. > > > > Usually, but we can't count on that, first mixer has 1 VI and 3 UI planes > > and second mixer has 1 VI and 1 UI plane. > > Right. So that would be easy to support using a property as well. The > only difference would be the CSC base. > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
On Tue, Nov 28, 2017 at 11:33:44PM +0100, Jernej Škrabec wrote: > Hi! > > Dne torek, 28. november 2017 ob 23:00:14 CET je Maxime Ripard napisal(a): > > On Tue, Nov 28, 2017 at 04:48:55PM +0100, Jernej Škrabec wrote: > > > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > > > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard > > > > > > napisal(a): > > > > > > Add support for the A83T display pipeline. > > > > > > > > > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > > > --- > > > > > > > > > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 3 > > > > > > +++ > > > > > > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 > > > > > > ++ > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 5 > > > > > > +++++ > > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | 4 > > > > > > ++++ > > > > > > 4 files changed, 14 insertions(+) > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > index > > > > > > d4259a4f5171..d6b52e5c48c0 100644 > > > > > > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > > > > > > @@ -93,6 +93,7 @@ Required properties: > > > > > > * allwinner,sun6i-a31s-tcon > > > > > > * allwinner,sun7i-a20-tcon > > > > > > * allwinner,sun8i-a33-tcon > > > > > > > > > > > > + * allwinner,sun8i-a83t-tcon-lcd > > > > > > > > > > > > * allwinner,sun8i-v3s-tcon > > > > > > > > > > > > - reg: base address and size of memory-mapped region > > > > > > - interrupts: interrupt associated to this IP > > > > > > > > > > > > @@ -224,6 +225,7 @@ supported. > > > > > > > > > > > > Required properties: > > > > > > - compatible: value must be one of: > > > > > > + * allwinner,sun8i-a83t-de2-mixer > > > > > > > > > > What will be the name of the second mixer, once support for HDMI is > > > > > added? > > > > > Should we start directly with 0 and 1 postfix ? > > > > > > > > What are the differences exactly without the two mixers? > > > > > > Mixer properties: > > > - mixer index (0 or 1), important for determining CCSC base (see my > > > patches) > > Is that the only thing we need to determine? > > For now, mixer index is important only for determining CCSC base in conjuction > with VEP capability. Obviously, I can't exclude that there is some other case > where that mixer index is needed. That's unfortunate... > Can't we just add reg property for that? No, reg is here specifically for the bus address, not for an index, and in general, indices are poorly perceived and have been subject to a lot of debate in the past. Hence why I'd really like to avoid any solution looking like this. But I guess we don't really have the choice either. Maxime
Dne četrtek, 30. november 2017 ob 16:33:12 CET je Maxime Ripard napisal(a): > On Tue, Nov 28, 2017 at 11:33:44PM +0100, Jernej Škrabec wrote: > > Hi! > > > > Dne torek, 28. november 2017 ob 23:00:14 CET je Maxime Ripard napisal(a): > > > On Tue, Nov 28, 2017 at 04:48:55PM +0100, Jernej Škrabec wrote: > > > > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote: > > > > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard > > > > > > > > napisal(a): > > > > > > > Add support for the A83T display pipeline. > > > > > > > > > > > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > > > > > > --- > > > > > > > > > > > > > > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | > > > > > > > 3 > > > > > > > +++ > > > > > > > drivers/gpu/drm/sun4i/sun4i_drv.c | > > > > > > > 2 > > > > > > > ++ > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | > > > > > > > 5 > > > > > > > +++++ > > > > > > > drivers/gpu/drm/sun4i/sun8i_mixer.c | > > > > > > > 4 > > > > > > > ++++ > > > > > > > 4 files changed, 14 insertions(+) > > > > > > > > > > > > > > diff --git > > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > index > > > > > > > d4259a4f5171..d6b52e5c48c0 100644 > > > > > > > --- > > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > +++ > > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > > > > > > > > > > > > > > @@ -93,6 +93,7 @@ Required properties: > > > > > > > * allwinner,sun6i-a31s-tcon > > > > > > > * allwinner,sun7i-a20-tcon > > > > > > > * allwinner,sun8i-a33-tcon > > > > > > > > > > > > > > + * allwinner,sun8i-a83t-tcon-lcd > > > > > > > > > > > > > > * allwinner,sun8i-v3s-tcon > > > > > > > > > > > > > > - reg: base address and size of memory-mapped region > > > > > > > - interrupts: interrupt associated to this IP > > > > > > > > > > > > > > @@ -224,6 +225,7 @@ supported. > > > > > > > > > > > > > > Required properties: > > > > > > > - compatible: value must be one of: > > > > > > > + * allwinner,sun8i-a83t-de2-mixer > > > > > > > > > > > > What will be the name of the second mixer, once support for HDMI > > > > > > is > > > > > > added? > > > > > > Should we start directly with 0 and 1 postfix ? > > > > > > > > > > What are the differences exactly without the two mixers? > > > > > > > > Mixer properties: > > > > - mixer index (0 or 1), important for determining CCSC base (see my > > > > patches) > > > > > > Is that the only thing we need to determine? > > > > For now, mixer index is important only for determining CCSC base in > > conjuction with VEP capability. Obviously, I can't exclude that there is > > some other case where that mixer index is needed. > > That's unfortunate... I take a deeper look today about that issue and it seems that mixer index is really needed only here. I did only regex search and not line by line check, so take this with grain of salt. Additionally, it seems that this comparison is kind of a hack for V3s. Channel output CSC is done through VEP (video enhancement processor, maybe?) unit. Those units are only on VI channels. But every SoC with DE2 I checked except V3s have 1 VI channel and VEP enabled on first mixer. V3s on the other hand have 2 VI channels and no VEP, but same addresses should work for channel output CSC according to the code. Of course, that is only a theory, but if you want, we can go with quirks structure. Or better yet, with quirk property, since there may be some (future?) SoC with two mixers and no VEP unit supported. BTW, how should be DE3 on H6 handled in the future? Registers offsets seems to be same within same DE2 and DE3 units, just extended. However, some base addresses are different, for example, those connected to VEP. I don't think new driver is needed, just some quirks structure which would tell DE version. Best regards, Jernej > > > Can't we just add reg property for that? > > No, reg is here specifically for the bus address, not for an index, > and in general, indices are poorly perceived and have been subject to > a lot of debate in the past. Hence why I'd really like to avoid any > solution looking like this. But I guess we don't really have the > choice either. > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt index d4259a4f5171..d6b52e5c48c0 100644 --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt @@ -93,6 +93,7 @@ Required properties: * allwinner,sun6i-a31s-tcon * allwinner,sun7i-a20-tcon * allwinner,sun8i-a33-tcon + * allwinner,sun8i-a83t-tcon-lcd * allwinner,sun8i-v3s-tcon - reg: base address and size of memory-mapped region - interrupts: interrupt associated to this IP @@ -224,6 +225,7 @@ supported. Required properties: - compatible: value must be one of: + * allwinner,sun8i-a83t-de2-mixer * allwinner,sun8i-v3s-de2-mixer - reg: base address and size of the memory-mapped region. - clocks: phandles to the clocks feeding the mixer @@ -253,6 +255,7 @@ Required properties: * allwinner,sun6i-a31s-display-engine * allwinner,sun7i-a20-display-engine * allwinner,sun8i-a33-display-engine + * allwinner,sun8i-a83t-display-engine * allwinner,sun8i-v3s-display-engine - allwinner,pipelines: list of phandle to the display engine diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index 75c76cdd82bc..c418be2f22be 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -193,6 +193,7 @@ static bool sun4i_drv_node_is_tcon(struct device_node *node) of_device_is_compatible(node, "allwinner,sun6i-a31s-tcon") || of_device_is_compatible(node, "allwinner,sun7i-a20-tcon") || of_device_is_compatible(node, "allwinner,sun8i-a33-tcon") || + of_device_is_compatible(node, "allwinner,sun8i-a83t-tcon-lcd") || of_device_is_compatible(node, "allwinner,sun8i-v3s-tcon"); } @@ -353,6 +354,7 @@ static const struct of_device_id sun4i_drv_of_table[] = { { .compatible = "allwinner,sun6i-a31s-display-engine" }, { .compatible = "allwinner,sun7i-a20-display-engine" }, { .compatible = "allwinner,sun8i-a33-display-engine" }, + { .compatible = "allwinner,sun8i-a83t-display-engine" }, { .compatible = "allwinner,sun8i-v3s-display-engine" }, { } }; diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 46ce6daa0b1a..871df75793a9 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1132,6 +1132,10 @@ static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_lvds_pll = true, }; +static const struct sun4i_tcon_quirks sun8i_a83t_quirks = { + /* nothing is supported */ +}; + static const struct sun4i_tcon_quirks sun8i_v3s_quirks = { /* nothing is supported */ }; @@ -1143,6 +1147,7 @@ static const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks }, { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, + { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_quirks }, { .compatible = "allwinner,sun8i-v3s-tcon", .data = &sun8i_v3s_quirks }, { } }; diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 44d5e639ebb2..5a1376965270 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -395,6 +395,10 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = { static const struct of_device_id sun8i_mixer_of_table[] = { { + .compatible = "allwinner,sun8i-a83t-de2-mixer", + .data = &sun8i_v3s_mixer_cfg, + }, + { .compatible = "allwinner,sun8i-v3s-de2-mixer", .data = &sun8i_v3s_mixer_cfg, },