[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Maxime Ripard]

On Thu, Dec 01, 2016 at 11:19:56AM +0200, Laurent Pinchart wrote:
> Hi Maxime,
> 
> On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
> > On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> > >> More, it is not sure that the bridge/DW code would work with Allwinner's
> > >> SoCs.
> > > 
> > > If it doesn't work and can't be made to work in a non-invasive way they it
> > > should certainly not be used :-)
> > 
> > Even if the register layout is completely scrambled, as long as the
> > bits themselves aren't (and by comparing the two drivers it looks like
> > they haven't changed), you can easily deal with that using the
> > regmap_fields, with the two implementations (the original one and the
> > scrambled one) providing their register map that way, and the driver
> > code using whatever has been provided.
> 
> Looking at https://linux-sunxi.org/DWC_HDMI_Controller#DWC_HDMI_Controller it 
> seems that an address remapping function could be used.

Even better.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Laurent Pinchart]

Hi Maxime,

On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
> On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> >> More, it is not sure that the bridge/DW code would work with Allwinner's
> >> SoCs.
> > 
> > If it doesn't work and can't be made to work in a non-invasive way they it
> > should certainly not be used :-)
> 
> Even if the register layout is completely scrambled, as long as the
> bits themselves aren't (and by comparing the two drivers it looks like
> they haven't changed), you can easily deal with that using the
> regmap_fields, with the two implementations (the original one and the
> scrambled one) providing their register map that way, and the driver
> code using whatever has been provided.

Looking at https://linux-sunxi.org/DWC_HDMI_Controller#DWC_HDMI_Controller it 
seems that an address remapping function could be used.

> >> Eventually, I went the same way as omap/hdmi5: different driver.
> > 
> > I might try to fix that for OMAP5 at some point, we'll see.
> 
> For complex drivers that have already a driver written and a lot of
> testing that already happened, I don't think duplication is a smart
> move.
> 
> >>>   - And finally the fact that we can't have several display engine in
> >>> parallel, if needs be. This has happened in the past already on
> >>> Allwinner SoCs, so it's definitely something we should consider in
> >>> the DT bindings, since we can't break them.
> >> 
> >> IIRC, I proposed my driver before yours, and the DE2 is completely
> >> different from the other display engines.
> >> What you are telling is "add more code to already complex code and have
> >> a big driver for all SoCs in each kernels".
> >> I think it should be better to have small modules, each one treating
> >> specific hardware, and to let only the needed code in the kernel memory
> >> at startup time.
> >> 
> >>> Until those are fixed, I cannot see how this driver can be merged,
> >>> unfortunately.
> >> 
> >> No problem. I just wanted to help people by giving the job I did on the
> >> boards I have. My boards are working for almost one year, fine enough
> >> for I use them as daily desktop computers. I don't want to spend one
> >> more year for having my code in the Linux kernel: there are so much
> >> other exciting things to do...
> > 
> > And you're certainly welcome to contribute drivers to the kernel, that's
> > always appreciated. Of course, to ensure a reasonable level of quality and
> > consistency between drivers, the review process often requires changes to
> > be made to the code being submitted. When it comes to drivers I mostly
> > pay attention to DT bindings, userspace APIs and modification to common
> > code. Driver code itself, as long as it's reasonably clean and doesn't
> > impede development of other drivers or impact system security in an
> > adverse way, is still important but maybe slightly less so. I'll defer to
> > Maxime to come to an agreement on the multiple display engines in
> > parallel problem as I'm not familiar with it for the Allwinner platforms.
> 
> The earlier Allwinner SoCs (with the old display engine), we had some
> SoCs with multiple instances of the display engine and TCON (the
> display engine roughly implementing the planes, the TCON the
> CRTC. Roughly.). However, those were sharing some encoders (HDMI,
> DSI) after that.
> 
> So we need to have a single DRM device taking care of the multiple
> display engines, which essentialy means that we have to decouple the
> DRM device from the display engine. This was done in the earlier
> designs using an additional node with a list of phandles to the
> display engines in the system, and obviously, I'd prefer to have some
> consistency and reuse the same thing.
> 
> But the current approach doesn't work and will require some DT
> modifications if that case happens again, which we can't do because of
> the DT ABI.

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Maxime Ripard]

Hi Laurent,

On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> > More, it is not sure that the bridge/DW code would work with Allwinner's
> > SoCs.
> 
> If it doesn't work and can't be made to work in a non-invasive way they it 
> should certainly not be used :-)

Even if the register layout is completely scrambled, as long as the
bits themselves aren't (and by comparing the two drivers it looks like
they haven't changed), you can easily deal with that using the
regmap_fields, with the two implementations (the original one and the
scrambled one) providing their register map that way, and the driver
code using whatever has been provided.

> > Eventually, I went the same way as omap/hdmi5: different driver.
> 
> I might try to fix that for OMAP5 at some point, we'll see.

For complex drivers that have already a driver written and a lot of
testing that already happened, I don't think duplication is a smart
move.

> > >   - And finally the fact that we can't have several display engine in
> > > parallel, if needs be. This has happened in the past already on
> > > Allwinner SoCs, so it's definitely something we should consider in
> > > the DT bindings, since we can't break them.
> > 
> > IIRC, I proposed my driver before yours, and the DE2 is completely
> > different from the other display engines.
> > What you are telling is "add more code to already complex code and have
> > a big driver for all SoCs in each kernels".
> > I think it should be better to have small modules, each one treating
> > specific hardware, and to let only the needed code in the kernel memory
> > at startup time.
> > 
> > > Until those are fixed, I cannot see how this driver can be merged,
> > > unfortunately.
> > 
> > No problem. I just wanted to help people by giving the job I did on the
> > boards I have. My boards are working for almost one year, fine enough
> > for I use them as daily desktop computers. I don't want to spend one
> > more year for having my code in the Linux kernel: there are so much
> > other exciting things to do...
> 
> And you're certainly welcome to contribute drivers to the kernel, that's 
> always appreciated. Of course, to ensure a reasonable level of quality and 
> consistency between drivers, the review process often requires changes to be 
> made to the code being submitted. When it comes to drivers I mostly pay 
> attention to DT bindings, userspace APIs and modification to common code. 
> Driver code itself, as long as it's reasonably clean and doesn't impede 
> development of other drivers or impact system security in an adverse way, is 
> still important but maybe slightly less so. I'll defer to Maxime to come to 
> an 
> agreement on the multiple display engines in parallel problem as I'm not 
> familiar with it for the Allwinner platforms.

The earlier Allwinner SoCs (with the old display engine), we had some
SoCs with multiple instances of the display engine and TCON (the
display engine roughly implementing the planes, the TCON the
CRTC. Roughly.). However, those were sharing some encoders (HDMI,
DSI) after that.

So we need to have a single DRM device taking care of the multiple
display engines, which essentialy means that we have to decouple the
DRM device from the display engine. This was done in the earlier
designs using an additional node with a list of phandles to the
display engines in the system, and obviously, I'd prefer to have some
consistency and reuse the same thing.

But the current approach doesn't work and will require some DT
modifications if that case happens again, which we can't do because of
the DT ABI.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Jernej Skrabec]

Hi Laurent,

Dne sreda, 30. november 2016 09.08.22 UTC+1 je oseba Laurent Pinchart 
napisala:
>
> Hi Jernej, 
>
> On Tuesday 29 Nov 2016 15:24:25 Jernej Skrabec wrote: 
> > Dne torek, 29. november 2016 23.56.31 UTC+1 je oseba Laurent Pinchart 
> > napisala: 
> > > On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote: 
> > >> Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard 
> > > napisala: 
> > >>> On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote: 
> >  This patchset series adds HDMI video support to the Allwinner 
> >  sun8i SoCs which include the display engine 2 (DE2). 
> >  The driver contains the code for the A83T and H3 SoCs, and 
> >  some H3 boards, but it could be used/extended for other SoCs 
> >  (A64, H2, H5) and boards (Banana PIs, Orange PIs). 
> > >>> 
> > >>> Honestly, I'm getting a bit worried by the fact that you ignore 
> > >>> reviews. 
> > >>> 
> > >>> On the important reviews that you got that are to be seen as major 
> > >>> issues that block the inclusion, we have: 
> > >>>   - The fact that the HDMI driver is actually just a designware IP, 
> > >>> and while you should use the driver that already exists, you 
> just 
> > >>> duplicated all that code. 
> > >> 
> > >> That might be hard thing to do. A83T fits perfectly, but H3 and newer 
> > >> SoCs do not. They are using completely different HDMI phy. Decoupling 
> > >> controller and phy code means rewritting a good portion of the code, 
> > >> unless some tricks are applied, like calling phy function pointers, 
> if 
> > >> they are defined. 
> > > 
> > > Same HDMI TX but different HDMI TX PHY ? Kieran is working on 
> decoupling 
> > > the PHY configuration code for a Renesas SoC, that might be of 
> interest to 
> > > you. 
> > 
> > Exactly. I'm developing only U-Boot driver, but Jean-Francois will 
> probably 
> > have more interest in this. 
>
> We'll post patches as soon as they're ready. 
>

Great. Is datasheet public? I'm curious if HDMI PHY is by any chance 
similar.
 

>
> By the way, do you know if the H3 and newer SoCs use a different PHY from 
> Synopsys, or a custom PHY developed by Allwinner ? 
>
>
Unfortunatelly, noone managed to identify PHY and Alliwinner never released 
a
bit of information about HDMI. Does config2_id code 0xFE (PHY type) tell you
anything?
 

> > >> Register addresses also differ, but that can be easily solved by 
> using 
> > >> undocumented magic value to restore them. 
> > > 
> > > I love that :-) 
> > 
> > Is it allowed to use magic number which was found in binary blob? I'm 
> new in 
> > all this. 
>
> I don't really see a problem with that, we have many drivers in the kernel 
> that have been developed through reverse-engineering. You should not 
> include 
> large pieces of code that have been obtained through decompilation of a 
> proprietary binary blob as those could be protected by copyright, but 
> writing 
> to undocumented registers based on information found through usage of a 
> binary 
> driver isn't a problem. (Please remember that I'm not a lawyer though) 
>
> > >>>   - The fact that you ignored Rob (v6) and I (v5) comment on using 
> OF 
> > >>> graph to model the connection between the display engine and the 
> > >>> TCON. Something that Laurent also pointed out in this version. 
> > >>>   
> > >>>   - The fact that you ignored that you needed an HDMI connector node 
> > >>> as a child of the HDMI controller. This has been reported by Rob 
> > >>> (v6) and yet again in this version by Laurent. 
> > >>>   
> > >>>   - And finally the fact that we can't have several display engine 
> in 
> > >>> parallel, if needs be. This has happened in the past already on 
> > >>> Allwinner SoCs, so it's definitely something we should consider 
> in 
> > >>> the DT bindings, since we can't break them. 
> > >>> 
> > >>> Until those are fixed, I cannot see how this driver can be merged, 
> > >>> unfortunately. 
>

Best regards,
Jernej Škrabec

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Laurent Pinchart]

Hi Jean-François,

On Wednesday 30 Nov 2016 10:05:45 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 22:36:50 +0100 Maxime Ripard wrote:
> > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> > > This patchset series adds HDMI video support to the Allwinner
> > > sun8i SoCs which include the display engine 2 (DE2).
> > > The driver contains the code for the A83T and H3 SoCs, and
> > > some H3 boards, but it could be used/extended for other SoCs
> > > (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> > 
> > Honestly, I'm getting a bit worried by the fact that you ignore
> > reviews.
> > 
> > On the important reviews that you got that are to be seen as major
> > 
> > issues that block the inclusion, we have:
> >   - The fact that the HDMI driver is actually just a designware IP,
> > and while you should use the driver that already exists, you just
> > duplicated all that code.
> 
> The DW registers in the A83T and H3 are obfuscated, so, the code in
> bridge/DW cannot be used as it is. There should be either a translation
> table or a function to compute the register addresses.

Jernej mentioned there could be a way to restore the original Synopsys memory 
map. If that works then using the dw-hdmi could be an option.

> More, it is not sure that the bridge/DW code would work with Allwinner's
> SoCs.

If it doesn't work and can't be made to work in a non-invasive way they it 
should certainly not be used :-)

> It seems that they got some schematics from DesignWare, but, is it really
> the same hardware?

That's not really relevant, as long as the hardware is compatible, it doesn't 
matter if it's a Synopsys IP or a custom implementation of the HDMI TX with a 
compatible interface.

> Also, if some changes had to be done in the bridge code, I could not check
> if this would break or not the other SoCs.

Nothing new here, all drivers that support multiple SoCs have the same 
problem. That's why we have maintainers, testers and board farms to try and 
catch issues as early as possible.

> Eventually, I went the same way as omap/hdmi5: different driver.

I might try to fix that for OMAP5 at some point, we'll see.

> >   - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> > graph to model the connection between the display engine and the
> > TCON. Something that Laurent also pointed out in this version.
> 
> I simply use the drm function drm_of_component_probe().
> If this one is in the DRM core, why should I not use it?
> If it must not be used, it would be nice to mark it as deprecated and
> to update the code of the drivers which are using it.

I haven't used that function so I can't comment here, except for the fact that 
DT bindings are not designed based on a given OS implementation. It should be 
the other way around, the bindings should be designed to clearly describe the 
hardware in a clean and consistent way that can be used by any host software, 
and the Linux helper functions should then be developed based on those 
bindings.

> >   - The fact that you ignored that you needed an HDMI connector node
> > as a child of the HDMI controller. This has been reported by Rob
> > (v6) and yet again in this version by Laurent.
> 
> As I don't know what is a DT 'connector', I cannot go further.
> I hope Laurent will give me clearer explanations and a real example.

Done, we can discuss this in that part of the mail thread.

> >   - And finally the fact that we can't have several display engine in
> > parallel, if needs be. This has happened in the past already on
> > Allwinner SoCs, so it's definitely something we should consider in
> > the DT bindings, since we can't break them.
> 
> IIRC, I proposed my driver before yours, and the DE2 is completely
> different from the other display engines.
> What you are telling is "add more code to already complex code and have
> a big driver for all SoCs in each kernels".
> I think it should be better to have small modules, each one treating
> specific hardware, and to let only the needed code in the kernel memory
> at startup time.
> 
> > Until those are fixed, I cannot see how this driver can be merged,
> > unfortunately.
> 
> No problem. I just wanted to help people by giving the job I did on the
> boards I have. My boards are working for almost one year, fine enough
> for I use them as daily desktop computers. I don't want to spend one
> more year for having my code in the Linux kernel: there are so much
> other exciting things to do...

And you're certainly welcome to contribute drivers to the kernel, that's 
always appreciated. Of course, to ensure a reasonable level of quality and 
consistency between drivers, the review process often requires changes to be 
made to the code being submitted. When it comes to drivers I mostly pay 
attention to DT bindings, userspace APIs and modification to common code. 
Driver code itself, as long as it's reasonably clean and doesn't impede 
development of 

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Jean-Francois Moine]

On Tue, 29 Nov 2016 22:36:50 +0100
Maxime Ripard  wrote:

> On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> > This patchset series adds HDMI video support to the Allwinner
> > sun8i SoCs which include the display engine 2 (DE2).
> > The driver contains the code for the A83T and H3 SoCs, and
> > some H3 boards, but it could be used/extended for other SoCs
> > (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> 
> Honestly, I'm getting a bit worried by the fact that you ignore
> reviews.
> 
> On the important reviews that you got that are to be seen as major
> issues that block the inclusion, we have:
>   - The fact that the HDMI driver is actually just a designware IP,
> and while you should use the driver that already exists, you just
> duplicated all that code.

The DW registers in the A83T and H3 are obfuscated, so, the code in
bridge/DW cannot be used as it is. There should be either a translation
table or a function to compute the register addresses.

More, it is not sure that the bridge/DW code would work with
Allwinner's SoCs. It seems that they got some schematics from
DesignWare, but, is it really the same hardware? Also, if some changes
had to be done in the bridge code, I could not check if this would
break or not the other SoCs.

Eventually, I went the same way as omap/hdmi5: different driver.

>   - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> graph to model the connection between the display engine and the
> TCON. Something that Laurent also pointed out in this version.

I simply use the drm function drm_of_component_probe().
If this one is in the DRM core, why should I not use it?
If it must not be used, it would be nice to mark it as deprecated and
to update the code of the drivers which are using it.

>   - The fact that you ignored that you needed an HDMI connector node
> as a child of the HDMI controller. This has been reported by Rob
> (v6) and yet again in this version by Laurent.

As I don't know what is a DT 'connector', I cannot go further.
I hope Laurent will give me clearer explanations and a real example.

>   - And finally the fact that we can't have several display engine in
> parallel, if needs be. This has happened in the past already on
> Allwinner SoCs, so it's definitely something we should consider in
> the DT bindings, since we can't break them.

IIRC, I proposed my driver before yours, and the DE2 is completely
different from the other display engines.
What you are telling is "add more code to already complex code and have
a big driver for all SoCs in each kernels".
I think it should be better to have small modules, each one treating
specific hardware, and to let only the needed code in the kernel memory
at startup time.

> Until those are fixed, I cannot see how this driver can be merged,
> unfortunately.

No problem. I just wanted to help people by giving the job I did on the
boards I have. My boards are working for almost one year, fine enough
for I use them as daily desktop computers. I don't want to spend one
more year for having my code in the Linux kernel: there are so much
other exciting things to do...

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Laurent Pinchart]

Hi Jernej,

On Tuesday 29 Nov 2016 15:24:25 Jernej Skrabec wrote:
> Dne torek, 29. november 2016 23.56.31 UTC+1 je oseba Laurent Pinchart
> napisala:
> > On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote:
> >> Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard
> > napisala:
> >>> On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
>  This patchset series adds HDMI video support to the Allwinner
>  sun8i SoCs which include the display engine 2 (DE2).
>  The driver contains the code for the A83T and H3 SoCs, and
>  some H3 boards, but it could be used/extended for other SoCs
>  (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> >>> 
> >>> Honestly, I'm getting a bit worried by the fact that you ignore
> >>> reviews.
> >>> 
> >>> On the important reviews that you got that are to be seen as major
> >>> issues that block the inclusion, we have:
> >>>   - The fact that the HDMI driver is actually just a designware IP,
> >>> and while you should use the driver that already exists, you just
> >>> duplicated all that code.
> >> 
> >> That might be hard thing to do. A83T fits perfectly, but H3 and newer
> >> SoCs do not. They are using completely different HDMI phy. Decoupling
> >> controller and phy code means rewritting a good portion of the code,
> >> unless some tricks are applied, like calling phy function pointers, if
> >> they are defined.
> > 
> > Same HDMI TX but different HDMI TX PHY ? Kieran is working on decoupling
> > the PHY configuration code for a Renesas SoC, that might be of interest to
> > you.
> 
> Exactly. I'm developing only U-Boot driver, but Jean-Francois will probably
> have more interest in this.

We'll post patches as soon as they're ready.

By the way, do you know if the H3 and newer SoCs use a different PHY from 
Synopsys, or a custom PHY developed by Allwinner ?

> >> Register addresses also differ, but that can be easily solved by using
> >> undocumented magic value to restore them.
> > 
> > I love that :-)
> 
> Is it allowed to use magic number which was found in binary blob? I'm new in
> all this.

I don't really see a problem with that, we have many drivers in the kernel 
that have been developed through reverse-engineering. You should not include 
large pieces of code that have been obtained through decompilation of a 
proprietary binary blob as those could be protected by copyright, but writing 
to undocumented registers based on information found through usage of a binary 
driver isn't a problem. (Please remember that I'm not a lawyer though)

> >>>   - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> >>> graph to model the connection between the display engine and the
> >>> TCON. Something that Laurent also pointed out in this version.
> >>>   
> >>>   - The fact that you ignored that you needed an HDMI connector node
> >>> as a child of the HDMI controller. This has been reported by Rob
> >>> (v6) and yet again in this version by Laurent.
> >>>   
> >>>   - And finally the fact that we can't have several display engine in
> >>> parallel, if needs be. This has happened in the past already on
> >>> Allwinner SoCs, so it's definitely something we should consider in
> >>> the DT bindings, since we can't break them.
> >>> 
> >>> Until those are fixed, I cannot see how this driver can be merged,
> >>> unfortunately.

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Jernej Skrabec]

Hi Laurent,

Dne torek, 29. november 2016 23.56.31 UTC+1 je oseba Laurent Pinchart 
napisala:
>
> Hi Jernej, 
>
> (CC'ing Kieran Bingham) 
>
> On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote: 
> > Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard 
> napisala: 
> > > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote: 
> > >> This patchset series adds HDMI video support to the Allwinner 
> > >> sun8i SoCs which include the display engine 2 (DE2). 
> > >> The driver contains the code for the A83T and H3 SoCs, and 
> > >> some H3 boards, but it could be used/extended for other SoCs 
> > >> (A64, H2, H5) and boards (Banana PIs, Orange PIs). 
> > > 
> > > Honestly, I'm getting a bit worried by the fact that you ignore 
> > > reviews. 
> > > 
> > > On the important reviews that you got that are to be seen as major 
> > > 
> > > issues that block the inclusion, we have: 
> > >   - The fact that the HDMI driver is actually just a designware IP, 
> > > and while you should use the driver that already exists, you just 
> > > duplicated all that code. 
> > 
> > That might be hard thing to do. A83T fits perfectly, but H3 and newer 
> SoCs 
> > do not. They are using completely different HDMI phy. Decoupling 
> controller 
> > and phy code means rewritting a good portion of the code, unless some 
> tricks 
> > are applied, like calling phy function pointers, if they are defined. 
>
> Same HDMI TX but different HDMI TX PHY ? Kieran is working on decoupling 
> the 
> PHY configuration code for a Renesas SoC, that might be of interest to 
> you. 
>

Exactly. I'm developing only U-Boot driver, but Jean-Francois will probably
have more interest in this.
 

>
> > Register addresses also differ, but that can be easily solved by using 
> > undocumented magic value to restore them. 
>
> I love that :-) 
>
>
Is it allowed to use magic number which was found in binary blob? I'm new in
all this.
 

> > >   - The fact that you ignored Rob (v6) and I (v5) comment on using OF 
> > > graph to model the connection between the display engine and the 
> > > TCON. Something that Laurent also pointed out in this version. 
> > >   
> > >   - The fact that you ignored that you needed an HDMI connector node 
> > > as a child of the HDMI controller. This has been reported by Rob 
> > > (v6) and yet again in this version by Laurent. 
> > >   
> > >   - And finally the fact that we can't have several display engine in 
> > > parallel, if needs be. This has happened in the past already on 
> > > Allwinner SoCs, so it's definitely something we should consider in 
> > > the DT bindings, since we can't break them. 
> > > 
> > > Until those are fixed, I cannot see how this driver can be merged, 
> > > unfortunately. 
>
> -- 
> Regards, 
>
> Laurent Pinchart 
>
>
Best regards,
Jernej Škrabec 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Laurent Pinchart]

Hi Jernej,

(CC'ing Kieran Bingham)

On Tuesday 29 Nov 2016 14:47:20 Jernej Skrabec wrote:
> Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard napisala:
> > On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> >> This patchset series adds HDMI video support to the Allwinner
> >> sun8i SoCs which include the display engine 2 (DE2).
> >> The driver contains the code for the A83T and H3 SoCs, and
> >> some H3 boards, but it could be used/extended for other SoCs
> >> (A64, H2, H5) and boards (Banana PIs, Orange PIs).
> > 
> > Honestly, I'm getting a bit worried by the fact that you ignore
> > reviews.
> > 
> > On the important reviews that you got that are to be seen as major
> > 
> > issues that block the inclusion, we have:
> >   - The fact that the HDMI driver is actually just a designware IP,
> > and while you should use the driver that already exists, you just
> > duplicated all that code.
> 
> That might be hard thing to do. A83T fits perfectly, but H3 and newer SoCs
> do not. They are using completely different HDMI phy. Decoupling controller
> and phy code means rewritting a good portion of the code, unless some tricks
> are applied, like calling phy function pointers, if they are defined.

Same HDMI TX but different HDMI TX PHY ? Kieran is working on decoupling the 
PHY configuration code for a Renesas SoC, that might be of interest to you.

> Register addresses also differ, but that can be easily solved by using
> undocumented magic value to restore them.

I love that :-)

> >   - The fact that you ignored Rob (v6) and I (v5) comment on using OF
> > graph to model the connection between the display engine and the
> > TCON. Something that Laurent also pointed out in this version.
> >   
> >   - The fact that you ignored that you needed an HDMI connector node
> > as a child of the HDMI controller. This has been reported by Rob
> > (v6) and yet again in this version by Laurent.
> >   
> >   - And finally the fact that we can't have several display engine in
> > parallel, if needs be. This has happened in the past already on
> > Allwinner SoCs, so it's definitely something we should consider in
> > the DT bindings, since we can't break them.
> > 
> > Until those are fixed, I cannot see how this driver can be merged,
> > unfortunately.

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Jernej Skrabec]

Hi Maxime,

Dne torek, 29. november 2016 22.37.03 UTC+1 je oseba Maxime Ripard napisala:
>
> On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote: 
> > This patchset series adds HDMI video support to the Allwinner 
> > sun8i SoCs which include the display engine 2 (DE2). 
> > The driver contains the code for the A83T and H3 SoCs, and 
> > some H3 boards, but it could be used/extended for other SoCs 
> > (A64, H2, H5) and boards (Banana PIs, Orange PIs). 
>
> Honestly, I'm getting a bit worried by the fact that you ignore 
> reviews. 
>
> On the important reviews that you got that are to be seen as major 
> issues that block the inclusion, we have: 
>   - The fact that the HDMI driver is actually just a designware IP, 
> and while you should use the driver that already exists, you just 
> duplicated all that code. 
>
>  
That might be hard thing to do. A83T fits perfectly, but H3 and newer SoCs 
do
not. They are using completely different HDMI phy. Decoupling controller and
phy code means rewritting a good portion of the code, unless some tricks are
applied, like calling phy function pointers, if they are defined.

Register addresses also differ, but that can be easily solved by using
undocumented magic value to restore them.
 

>   - The fact that you ignored Rob (v6) and I (v5) comment on using OF 
> graph to model the connection between the display engine and the 
> TCON. Something that Laurent also pointed out in this version. 
>
>   - The fact that you ignored that you needed an HDMI connector node 
> as a child of the HDMI controller. This has been reported by Rob 
> (v6) and yet again in this version by Laurent. 
>
>   - And finally the fact that we can't have several display engine in 
> parallel, if needs be. This has happened in the past already on 
> Allwinner SoCs, so it's definitely something we should consider in 
> the DT bindings, since we can't break them. 
>
> Until those are fixed, I cannot see how this driver can be merged, 
> unfortunately. 
>
> Maxime 
>
> -- 
> Maxime Ripard, Free Electrons 
> Embedded Linux and Kernel engineering 
> http://free-electrons.com


Best regards,
Jernej Škrabec 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support

[Maxime Ripard]

On Tue, Nov 29, 2016 at 11:18:35AM +0100, Jean-Francois Moine wrote:
> This patchset series adds HDMI video support to the Allwinner
> sun8i SoCs which include the display engine 2 (DE2).
> The driver contains the code for the A83T and H3 SoCs, and
> some H3 boards, but it could be used/extended for other SoCs
> (A64, H2, H5) and boards (Banana PIs, Orange PIs).

Honestly, I'm getting a bit worried by the fact that you ignore
reviews.

On the important reviews that you got that are to be seen as major
issues that block the inclusion, we have:
  - The fact that the HDMI driver is actually just a designware IP,
and while you should use the driver that already exists, you just
duplicated all that code.

  - The fact that you ignored Rob (v6) and I (v5) comment on using OF
graph to model the connection between the display engine and the
TCON. Something that Laurent also pointed out in this version.

  - The fact that you ignored that you needed an HDMI connector node
as a child of the HDMI controller. This has been reported by Rob
(v6) and yet again in this version by Laurent.

  - And finally the fact that we can't have several display engine in
parallel, if needs be. This has happened in the past already on
Allwinner SoCs, so it's definitely something we should consider in
the DT bindings, since we can't break them.

Until those are fixed, I cannot see how this driver can be merged,
unfortunately.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature