| Message ID | 1377866264-21110-2-git-send-email-ulrich.hecht@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi Ulrich, Thank you for the patch. On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: > ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 branch > at https://github.com/analogdevicesinc/linux.git I believe Lars-Peter (CC'ed) was planning to upstream the driver. Lars-Peter, could you please share your plans ? > Changed to export its i2c_client handle via platform_data, which is the only > way I could come up with to use a DRM encoder that is not attached to a > dedicated on-GPU i2c bus. > > Signed-off-by: Ulrich Hecht <ulrich.hecht@gmail.com> > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/adv7511.h | 455 +++++++++++++++++ > drivers/gpu/drm/i2c/adv7511_audio.c | 304 +++++++++++ > drivers/gpu/drm/i2c/adv7511_core.c | 981 +++++++++++++++++++++++++++++++++ > 5 files changed, 1749 insertions(+) > create mode 100644 drivers/gpu/drm/i2c/adv7511.h > create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c
On 09/02/2013 03:18 PM, Laurent Pinchart wrote: > Hi Ulrich, > > Thank you for the patch. > > On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: >> ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 branch >> at https://github.com/analogdevicesinc/linux.git > > I believe Lars-Peter (CC'ed) was planning to upstream the driver. Lars-Peter, > could you please share your plans ? My plan was to have all this upstream long time ago ;) As I said in that other thread, I don't think it is a good idea to have two drivers for the same device. But if nobody else sees a problem with this and as long as the v4l driver doesn't have devicetree support I guess we could get away with it for now. Even if it will probably haunt us later on. There are still a few minor bits and pieces to iron out, but lets try to aim for 2.6.13. - Lars > >> Changed to export its i2c_client handle via platform_data, which is the only >> way I could come up with to use a DRM encoder that is not attached to a >> dedicated on-GPU i2c bus. >> >> Signed-off-by: Ulrich Hecht <ulrich.hecht@gmail.com> >> --- >> drivers/gpu/drm/Kconfig | 6 + >> drivers/gpu/drm/i2c/Makefile | 3 + >> drivers/gpu/drm/i2c/adv7511.h | 455 +++++++++++++++++ >> drivers/gpu/drm/i2c/adv7511_audio.c | 304 +++++++++++ >> drivers/gpu/drm/i2c/adv7511_core.c | 981 +++++++++++++++++++++++++++++++++ >> 5 files changed, 1749 insertions(+) >> create mode 100644 drivers/gpu/drm/i2c/adv7511.h >> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c >> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars-Peter, On Monday 02 September 2013 15:40:22 Lars-Peter Clausen wrote: > On 09/02/2013 03:18 PM, Laurent Pinchart wrote: > > On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: > >> > >> ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 > >> branch at https://github.com/analogdevicesinc/linux.git > > > > I believe Lars-Peter (CC'ed) was planning to upstream the driver. > > Lars-Peter, could you please share your plans ? > > My plan was to have all this upstream long time ago ;) > > As I said in that other thread, I don't think it is a good idea to have two > drivers for the same device. But if nobody else sees a problem with this and > as long as the v4l driver doesn't have devicetree support I guess we could > get away with it for now. Even if it will probably haunt us later on. > > There are still a few minor bits and pieces to iron out, but lets try to aim > for 2.6.13. If you can make it for 2.6.13 I will be extremely impressed :-) Do you plan to push the driver yourself ? If so, I would appreciate if you could CC me. If there's just a few minor bits to iron out I can already review your latest version if that can help.
On 09/02/2013 04:15 PM, Laurent Pinchart wrote: > Hi Lars-Peter, > > On Monday 02 September 2013 15:40:22 Lars-Peter Clausen wrote: >> On 09/02/2013 03:18 PM, Laurent Pinchart wrote: >>> On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: >>>> >>>> ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 >>>> branch at https://github.com/analogdevicesinc/linux.git >>> >>> I believe Lars-Peter (CC'ed) was planning to upstream the driver. >>> Lars-Peter, could you please share your plans ? >> >> My plan was to have all this upstream long time ago ;) >> >> As I said in that other thread, I don't think it is a good idea to have two >> drivers for the same device. But if nobody else sees a problem with this and >> as long as the v4l driver doesn't have devicetree support I guess we could >> get away with it for now. Even if it will probably haunt us later on. >> >> There are still a few minor bits and pieces to iron out, but lets try to aim >> for 2.6.13. > > If you can make it for 2.6.13 I will be extremely impressed :-) Yea, I know DRM always takes a bit longer... > > Do you plan to push the driver yourself ? If so, I would appreciate if you > could CC me. If there's just a few minor bits to iron out I can already review > your latest version if that can help. > There are a couple of style issues, so if you review the driver ignore these for now, but otherwise feedback is welcome, thanks. And I'm not also quite happy with the ASoC integration yet. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars-Peter, On Monday 02 September 2013 16:43:25 Lars-Peter Clausen wrote: > On 09/02/2013 04:15 PM, Laurent Pinchart wrote: > > On Monday 02 September 2013 15:40:22 Lars-Peter Clausen wrote: > >> On 09/02/2013 03:18 PM, Laurent Pinchart wrote: > >>> On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: > >>>> ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 > >>>> branch at https://github.com/analogdevicesinc/linux.git > >>> > >>> I believe Lars-Peter (CC'ed) was planning to upstream the driver. > >>> Lars-Peter, could you please share your plans ? > >> > >> My plan was to have all this upstream long time ago ;) > >> > >> As I said in that other thread, I don't think it is a good idea to have > >> two drivers for the same device. But if nobody else sees a problem with > >> this and as long as the v4l driver doesn't have devicetree support I > >> guess we could get away with it for now. Even if it will probably haunt > >> us later on. > >> > >> There are still a few minor bits and pieces to iron out, but lets try to > >> aim for 2.6.13. > > > > If you can make it for 2.6.13 I will be extremely impressed :-) > > Yea, I know DRM always takes a bit longer... I think you meant 3.13 ;-) > > Do you plan to push the driver yourself ? If so, I would appreciate if you > > could CC me. If there's just a few minor bits to iron out I can already > > review your latest version if that can help. > > There are a couple of style issues, so if you review the driver ignore these > for now, but otherwise feedback is welcome, thanks. And I'm not also quite > happy with the ASoC integration yet. Sure. Is the version available from the above branch the latest version ?
On 09/02/2013 05:01 PM, Laurent Pinchart wrote: > Hi Lars-Peter, > > On Monday 02 September 2013 16:43:25 Lars-Peter Clausen wrote: >> On 09/02/2013 04:15 PM, Laurent Pinchart wrote: >>> On Monday 02 September 2013 15:40:22 Lars-Peter Clausen wrote: >>>> On 09/02/2013 03:18 PM, Laurent Pinchart wrote: >>>>> On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: >>>>>> ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 >>>>>> branch at https://github.com/analogdevicesinc/linux.git >>>>> >>>>> I believe Lars-Peter (CC'ed) was planning to upstream the driver. >>>>> Lars-Peter, could you please share your plans ? >>>> >>>> My plan was to have all this upstream long time ago ;) >>>> >>>> As I said in that other thread, I don't think it is a good idea to have >>>> two drivers for the same device. But if nobody else sees a problem with >>>> this and as long as the v4l driver doesn't have devicetree support I >>>> guess we could get away with it for now. Even if it will probably haunt >>>> us later on. >>>> >>>> There are still a few minor bits and pieces to iron out, but lets try to >>>> aim for 2.6.13. >>> >>> If you can make it for 2.6.13 I will be extremely impressed :-) >> >> Yea, I know DRM always takes a bit longer... > > I think you meant 3.13 ;-) > >>> Do you plan to push the driver yourself ? If so, I would appreciate if you >>> could CC me. If there's just a few minor bits to iron out I can already >>> review your latest version if that can help. >> >> There are a couple of style issues, so if you review the driver ignore these >> for now, but otherwise feedback is welcome, thanks. And I'm not also quite >> happy with the ASoC integration yet. > > Sure. Is the version available from the above branch the latest version ? Yes, you can find it here: https://github.com/analogdevicesinc/linux/tree/xcomm_zynq/drivers/gpu/drm/i2c Oh and the DT bindings also still need proper documentation. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars-Peter, (CC'ing Hans Verkuil and the dri-devel and linux-media mailing lists) On Monday 02 September 2013 17:09:11 Lars-Peter Clausen wrote: > On 09/02/2013 05:01 PM, Laurent Pinchart wrote: > > On Monday 02 September 2013 16:43:25 Lars-Peter Clausen wrote: > >> On 09/02/2013 04:15 PM, Laurent Pinchart wrote: > >>> On Monday 02 September 2013 15:40:22 Lars-Peter Clausen wrote: > >>>> On 09/02/2013 03:18 PM, Laurent Pinchart wrote: > >>>>> On Friday 30 August 2013 14:37:35 Ulrich Hecht wrote: > >>>>>> ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 > >>>>>> branch at https://github.com/analogdevicesinc/linux.git > >>>>> > >>>>> I believe Lars-Peter (CC'ed) was planning to upstream the driver. > >>>>> Lars-Peter, could you please share your plans ? > >>>> > >>>> My plan was to have all this upstream long time ago ;) > >>>> > >>>> As I said in that other thread, I don't think it is a good idea to have > >>>> two drivers for the same device. But if nobody else sees a problem with > >>>> this and as long as the v4l driver doesn't have devicetree support I > >>>> guess we could get away with it for now. Even if it will probably haunt > >>>> us later on. > >>>> > >>>> There are still a few minor bits and pieces to iron out, but lets try > >>>> to aim for 2.6.13. > >>> > >>> If you can make it for 2.6.13 I will be extremely impressed :-) > >> > >> Yea, I know DRM always takes a bit longer... > > > > I think you meant 3.13 ;-) > > > >>> Do you plan to push the driver yourself ? If so, I would appreciate if > >>> you could CC me. If there's just a few minor bits to iron out I can > >>> already review your latest version if that can help. > >> > >> There are a couple of style issues, so if you review the driver ignore > >> these for now, but otherwise feedback is welcome, thanks. And I'm not > >> also quite happy with the ASoC integration yet. I'll thus concentrate on the video part in the review. Any chance to get the video portion to mainline first and then add audio support ? > > Sure. Is the version available from the above branch the latest version ? > > Yes, you can find it here: > https://github.com/analogdevicesinc/linux/tree/xcomm_zynq/drivers/gpu/drm/ > i2c Thank you. > Oh and the DT bindings also still need proper documentation. I've squashed all the patches together and have copied the result below. > From f7c27f204cab3d7dcaa128880c0d9ef1ae0e2fc6 Mon Sep 17 00:00:00 2001 > From: Lars-Peter Clausen <lars@metafoo.de> > Date: Mon, 5 Mar 2012 16:30:57 +0100 > Subject: [PATCH] DRM: Add adv7511 encoder driver > > This patch adds a driver for the Analog Devices adv7511. The adv7511 is a > standalone HDMI transmitter chip. It features a HDMI output interface on one > end and video and audio input interfaces on the other. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- > drivers/gpu/drm/Kconfig | 6 + > drivers/gpu/drm/i2c/Makefile | 3 + > drivers/gpu/drm/i2c/adv7511.h | 454 +++++++++++++++++ > drivers/gpu/drm/i2c/adv7511_audio.c | 304 +++++++++++ > drivers/gpu/drm/i2c/adv7511_core.c | 979 +++++++++++++++++++++++++++++++++ > 5 files changed, 1746 insertions(+) > create mode 100644 drivers/gpu/drm/i2c/adv7511.h > create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c First of all, please run checkpatch.pl on the code :-) > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 626bc0c..d8862a4 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -200,6 +200,12 @@ config DRM_SAVAGE > Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister > chipset. If M is selected the module will be called savage. > > +config DRM_ENCODER_ADV7511 > + tristate "AV7511 encoder" > + depends on DRM && I2C > + select REGMAP_I2C > + select HDMI Beside adding a help text, you should also depend on and/or select SND_SOC. > + > source "drivers/gpu/drm/exynos/Kconfig" > > source "drivers/gpu/drm/vmwgfx/Kconfig" [snip] > diff --git a/drivers/gpu/drm/i2c/adv7511.h b/drivers/gpu/drm/i2c/adv7511.h > new file mode 100644 > index 0000000..4631fcd6 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/adv7511.h > @@ -0,0 +1,454 @@ > +/** > + * Analog Devices ADV7511 HDMI transmitter driver > + * > + * Copyright 2012 Analog Devices Inc. > + * > + * Licensed under the GPL-2. > + */ > + > +#ifndef __ADV7511_H__ > +#define __ADV7511_H__ > + > +#include <linux/hdmi.h> This file should be split in two headers, one with platform data that will go to include/linux/platform_data/, and another one that can stay here. > +#define ADV7511_REG_CHIP_REVISION 0x00 > +#define ADV7511_REG_N0 0x01 > +#define ADV7511_REG_N1 0x02 > +#define ADV7511_REG_N2 0x03 > +#define ADV7511_REG_SPDIF_FREQ 0x04 > +#define ADV7511_REG_CTS_AUTOMATIC1 0x05 > +#define ADV7511_REG_CTS_AUTOMATIC2 0x06 > +#define ADV7511_REG_CTS_MANUAL0 0x07 > +#define ADV7511_REG_CTS_MANUAL1 0x08 > +#define ADV7511_REG_CTS_MANUAL2 0x09 [snip] > +#define ADV7511_CSC_ENABLE BIT(7) > +#define ADV7511_CSC_UPDATE_MODE BIT(5) I would have added the bits right after the register they refer to, but that's up to you. [snip] > +#include <drm/drmP.h> You can move the include at the beginning of the file. > +struct i2c_client; > +struct regmap; > +struct adv7511; Nitpicking, you could sort the structures alphabetically. > +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet); > +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet); > + > +int adv7511_audio_init(struct device *dev); > +void adv7511_audio_exit(struct device *dev); > + > +/** > + * enum adv7511_input_style - Selects the input format style > + * ADV7511_INPUT_STYLE1: Use input style 1 > + * ADV7511_INPUT_STYLE2: Use input style 2 > + * ADV7511_INPUT_STYLE3: Use input style 3 > + **/ > +enum adv7511_input_style { > + ADV7511_INPUT_STYLE1 = 2, > + ADV7511_INPUT_STYLE2 = 1, > + ADV7511_INPUT_STYLE3 = 3, > +}; > + > +/** > + * enum adv7511_input_id - Selects the input format id > + * @ADV7511_INPUT_ID_24BIT_RGB444_YCbCr444: Input pixel format is 24-bit > + * 444 RGB or 444 YCbCR with separate syncs > + * @ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_SEPARATE_SYNC: > + * @ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_EMBEDDED_SYNC: > + * @ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_SEPARATE_SYNC: > + * @ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_EMBEDDED_SYNC: > + * @ADV7511_INPUT_ID_12_15_16BIT_RGB444_YCbCr444: > + **/ > +enum adv7511_input_id { > + ADV7511_INPUT_ID_24BIT_RGB444_YCbCr444 = 0, > + ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_SEPARATE_SYNC = 1, > + ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_EMBEDDED_SYNC = 2, > + ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_SEPARATE_SYNC = 3, > + ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_EMBEDDED_SYNC = 4, > + ADV7511_INPUT_ID_12_15_16BIT_RGB444_YCbCr444 = 5, > +}; > + > +/** > + * enum adv7511_input_bit_justifiction - Selects the input format bit justifiction > + * ADV7511_INPUT_BIT_JUSTIFICATION_EVENLY: Input bits are evenly distributed > + * ADV7511_INPUT_BIT_JUSTIFICATION_RIGHT: Input bit signals have right justification > + * ADV7511_INPUT_BIT_JUSTIFICATION_LEFT: Input bit signals have left justification > + **/ > +enum adv7511_input_bit_justifiction { > + ADV7511_INPUT_BIT_JUSTIFICATION_EVENLY = 0, > + ADV7511_INPUT_BIT_JUSTIFICATION_RIGHT = 1, > + ADV7511_INPUT_BIT_JUSTIFICATION_LEFT = 2, > +}; > + > +/** > + * enum adv7511_input_color_depth - Selects the input format color depth > + * @ADV7511_INPUT_COLOR_DEPTH_8BIT: Input format color depth is 8 bits per channel > + * @ADV7511_INPUT_COLOR_DEPTH_10BIT: Input format color dpeth is 10 bits per channel > + * @ADV7511_INPUT_COLOR_DEPTH_12BIT: Input format color depth is 12 bits per channel > + **/ > +enum adv7511_input_color_depth { > + ADV7511_INPUT_COLOR_DEPTH_8BIT = 3, > + ADV7511_INPUT_COLOR_DEPTH_10BIT = 1, > + ADV7511_INPUT_COLOR_DEPTH_12BIT = 2, > +}; Those enums describe the video format at the chip input. This can be configured statically from platform data or DT, but some platforms might need to setup formats dynamically at runtime. For instance the ADV7511 could be driven by a mux with two inputs using different formats. For these reasons I would combine all those enums in a single one that lists all possible input formats. The formats should be standardized and moved to a separate header file. Get and set format operations will be needed (this is something CDF will address :-)). > + > +/** > + * enum adv7511_input_sync_pulse - Selects the sync pulse > + * @ADV7511_INPUT_SYNC_PULSE_DE: Use the DE signal as sync pulse > + * @ADV7511_INPUT_SYNC_PULSE_HSYNC: Use the HSYNC signal as sync pulse > + * @ADV7511_INPUT_SYNC_PULSE_VSYNC: Use the VSYNC signal as sync pulse > + * @ADV7511_INPUT_SYNC_PULSE_NONE: No external sync pulse signal > + **/ > +enum adv7511_input_sync_pulse { > + ADV7511_INPUT_SYNC_PULSE_DE = 0, > + ADV7511_INPUT_SYNC_PULSE_HSYNC = 1, > + ADV7511_INPUT_SYNC_PULSE_VSYNC = 2, > + ADV7511_INPUT_SYNC_PULSE_NONE = 3, > +}; This property should also be standardized. It might make sense to define new display flags (include/video/display_timing.h), but a separate enum is possible as well. > +/** > + * enum adv7511_input_clock_delay - Delay for the video data input clock > + * @ADV7511_INPUT_CLOCK_DELAY_MINUS_1200PS: -1200 pico seconds delay > + * @ADV7511_INPUT_CLOCK_DELAY_MINUS_800PS: -800 pico seconds delay > + * @ADV7511_INPUT_CLOCK_DELAY_MINUS_400PS: -400 pico seconds delay > + * @ADV7511_INPUT_CLOCK_DELAY_NONE: No delay > + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_400PS: 400 pico seconds delay > + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_800PS: 800 pico seconds delay > + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_1200PS: 1200 pico seconds delay > + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_1600PS: 1600 pico seconds delay > + **/ > +enum adv7511_input_clock_delay { > + ADV7511_INPUT_CLOCK_DELAY_MINUS_1200PS = 0, > + ADV7511_INPUT_CLOCK_DELAY_MINUS_800PS = 1, > + ADV7511_INPUT_CLOCK_DELAY_MINUS_400PS = 2, > + ADV7511_INPUT_CLOCK_DELAY_NONE = 3, > + ADV7511_INPUT_CLOCK_DELAY_PLUS_400PS = 4, > + ADV7511_INPUT_CLOCK_DELAY_PLUS_800PS = 5, > + ADV7511_INPUT_CLOCK_DELAY_PLUS_1200PS = 6, > + ADV7511_INPUT_CLOCK_DELAY_PLUS_1600PS = 7, > +}; > + > +/** > + * enum adv7511_sync_polarity - Polarity for the input sync signals > + * ADV7511_SYNC_POLARITY_PASSTHROUGH: Sync polarity matches that of the > + * currently configured mode. > + * ADV7511_SYNC_POLARITY_LOW: Sync polarity is low > + * ADV7511_SYNC_POLARITY_HIGH: Sync polarity is high > + * > + * If the polarity is set to either ADV7511_SYNC_POLARITY_LOW or > + * ADV7511_SYNC_POLARITY_HIGH the ADV7511 will internally invert the signal > + * if it is required to match the sync polarity setting for the currently > + * selected mode. If the polarity is set to > + * ADV7511_SYNC_POLARITY_PASSTHROUGH, the ADV7511 will route the signal > + * unchanged, this is useful if the upstream graphics core will already > + * generate the sync singals with the correct polarity. > + **/ > +enum adv7511_sync_polarity { > + ADV7511_SYNC_POLARITY_PASSTHROUGH, > + ADV7511_SYNC_POLARITY_LOW, > + ADV7511_SYNC_POLARITY_HIGH, > +}; This property should be standardized as well. > +/** > + * enum adv7511_timing_gen_seq - Selects the order in which timing adjustments are performed > + * @ADV7511_TIMING_GEN_SEQ_SYN_ADJ_FIRST: Sync adjustment first, then DE generation > + * @ADV7511_TIMING_GEN_SEQ_DE_GEN_FIRST: DE generation first, then sync adjustment > + * > + * This setting is only relevant if both DE generation and sync adjustment are > + * active. > + **/ > +enum adv7511_timing_gen_seq { > + ADV7511_TIMING_GEN_SEQ_SYN_ADJ_FIRST = 0, > + ADV7511_TIMING_GEN_SEQ_DE_GEN_FIRST = 1, > +}; > + > +/** > + * enum adv7511_up_conversion - Selects the upscaling conversion method > + * @ADV7511_UP_CONVERSION_ZERO_ORDER: Use zero order up conversion > + * @ADV7511_UP_CONVERSION_FIRST_ORDER: Use first order up conversion > + * > + * This used when converting from a 4:2:2 format to a 4:4:4 format. > + **/ > +enum adv7511_up_conversion { > + ADV7511_UP_CONVERSION_ZERO_ORDER = 0, > + ADV7511_UP_CONVERSION_FIRST_ORDER = 1, > +}; How is the upscaling conversion method supposed to be selected ? What does it depend on ? > +/** > + * struct adv7511_link_config - Describes adv7511 hardware configuration > + * @id: Video input format id > + * @input_style: Video input format style > + * @sync_pulse: Select the sync pulse > + * @clock_delay: Clock delay for the input clock > + * @reverse_bitorder: Reverse video input signal bitorder > + * @bit_justification: Video input format bit justification > + * @up_conversion: Selects the upscaling conversion method > + * @input_color_depth: Input video format color depth > + * @tmds_clock_inversion: Whether to invert the TDMS clock > + * @vsync_polartity: vsync input signal configuration > + * @hsync_polartity: hsync input signal configuration > + * @timing_gen_seq: Selects the order in which sync DE generation > + * and sync adjustment are performt. > + * @gpio_pd: GPIO controlling the PD (powerdown) pin > + **/ > +struct adv7511_link_config { > + enum adv7511_input_id id; > + enum adv7511_input_style input_style; > + enum adv7511_input_sync_pulse sync_pulse; > + enum adv7511_input_clock_delay clock_delay; > + bool reverse_bitorder; > + enum adv7511_input_bit_justifiction bit_justification; > + enum adv7511_up_conversion up_conversion; > + enum adv7511_input_color_depth input_color_depth; > + bool tmds_clock_inversion; > + enum adv7511_timing_gen_seq timing_gen_seq; > + > + enum adv7511_sync_polarity vsync_polarity; > + enum adv7511_sync_polarity hsync_polarity; > + > + int gpio_pd; > +}; > + > +/** > + adi,input-style = 1|2|3; > + adi,input-id = > + "24-bit-rgb444-ycbcr444", > + "16-20-24-bit-ycbcr422-separate-sync" | > + "16-20-24-bit-ycbcr422-embedded-sync" | > + "8-10-12-bit-ycbcr422-separate-sync" | > + "8-10-12-bit-ycbcr422-embedded-sync" | > + "12-15-16-bit-rgb444-ycbcr444" > + adi,sync-pulse = "de","vsync","hsync","none" > + adi,clock-delay = -1200|-800|-400|0|400|800|1200|1600 > + adi,reverse-bitorder > + adi,bit-justification = "left"|"right"|"evently"; > + adi,up-conversion = "zero-order"|"first-order" > + adi,input-color-depth = 8|10|12 > + adi,tdms-clock-inversion > + adi,vsync-polarity = "low"|"high"|"passthrough" > + adi,hsync-polarity = "low"|"high"|"passtrhough" > + adi,timing-gen-seq = "sync-adjustment-first"|"de-generation-first" > +*/ > + > +/** > + * enum adv7511_csc_scaling - Scaling factor for the ADV7511 CSC > + * @ADV7511_CSC_SCALING_1: CSC results are not scaled > + * @ADV7511_CSC_SCALING_2: CSC results are scaled by a factor of two > + * @ADV7511_CSC_SCALING_4: CSC results are scalled by a factor of four > + **/ > +enum adv7511_csc_scaling { > + ADV7511_CSC_SCALING_1 = 0, > + ADV7511_CSC_SCALING_2 = 1, > + ADV7511_CSC_SCALING_4 = 2, > +}; > + > +/** > + * struct adv7511_video_config - Describes adv7511 hardware configuration > + * @csc_enable: Whether to enable color space conversion > + * @csc_scaling_factor: Color space conversion scaling factor > + * @csc_coefficents: Color space conversion coefficents > + * @hdmi_mode: Whether to use HDMI or DVI output mode > + * @avi_infoframe: HDMI infoframe > + **/ > +struct adv7511_video_config { > + bool csc_enable; Shouldn't this be automatically computed based on the input and output formats ? > + enum adv7511_csc_scaling csc_scaling_factor; > + const uint16_t *csc_coefficents; Do the coefficients need to be configured freely, or could presets do ? > + bool hdmi_mode; > + struct hdmi_avi_infoframe avi_infoframe; > +}; > + > +struct adv7511 { > + struct i2c_client *i2c_main; > + struct i2c_client *i2c_edid; > + struct i2c_client *i2c_packet; > + struct i2c_client *i2c_cec; > + > + struct regmap *regmap; > + struct regmap *packet_memory_regmap; > + enum drm_connector_status status; > + int dpms_mode; > + > + unsigned int f_tmds; > + unsigned int f_audio; > + > + unsigned int audio_source; > + > + unsigned int current_edid_segment; > + uint8_t edid_buf[256]; > + > + wait_queue_head_t wq; > + struct drm_encoder *encoder; > + > + bool embedded_sync; > + enum adv7511_sync_polarity vsync_polarity; > + enum adv7511_sync_polarity hsync_polarity; > + > + struct edid *edid; > + > + int gpio_pd; > +}; > + > +struct edid *adv7511_get_edid(struct drm_encoder *encoder); > + > +#endif > diff --git a/drivers/gpu/drm/i2c/adv7511_audio.c b/drivers/gpu/drm/i2c/adv7511_audio.c > new file mode 100644 > index 0000000..746f383 > --- /dev/null > +++ b/drivers/gpu/drm/i2c/adv7511_audio.c > @@ -0,0 +1,304 @@ > +/** > + * Analog Devices ADV7511 HDMI transmitter driver > + * > + * Copyright 2012 Analog Devices Inc. > + * > + * Licensed under the GPL-2. > + */ > + > +#include <linux/module.h> > +#include <linux/moduleparam.h> > +#include <linux/init.h> > +#include <linux/delay.h> > +#include <linux/pm.h> > +#include <linux/i2c.h> > +#include <linux/spi/spi.h> Is this header needed ? > +#include <linux/slab.h> > +#include <sound/core.h> > +#include <sound/pcm.h> > +#include <sound/pcm_params.h> > +#include <sound/soc.h> > +#include <sound/initval.h> > +#include <sound/tlv.h> What about sorting the headers alphabetically ? :-) > + > +#include "adv7511.h" [snip] > diff --git a/drivers/gpu/drm/i2c/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511_core.c > new file mode 100644 > index 0000000..f26151d > --- /dev/null > +++ b/drivers/gpu/drm/i2c/adv7511_core.c > @@ -0,0 +1,979 @@ > +/** > + * Analog Devices ADV7511 HDMI transmitter driver > + * > + * Copyright 2012 Analog Devices Inc. > + * > + * Licensed under the GPL-2. > + */ > + > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/gpio.h> > +#include <linux/of_gpio.h> > + > +#include "adv7511.h" > + > +#include <drm/drmP.h> > +#include <drm/drm_crtc_helper.h> > +#include <drm/drm_encoder_slave.h> > +#include <drm/drm_edid.h> Alphabetical order as well ? > +static const uint8_t adv7511_register_defaults[] = { > + 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 00 */ > + 0x00, 0x00, 0x01, 0x0e, 0xbc, 0x18, 0x01, 0x13, > + 0x25, 0x37, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 10 */ > + 0x46, 0x62, 0x04, 0xa8, 0x00, 0x00, 0x1c, 0x84, > + 0x1c, 0xbf, 0x04, 0xa8, 0x1e, 0x70, 0x02, 0x1e, /* 20 */ > + 0x00, 0x00, 0x04, 0xa8, 0x08, 0x12, 0x1b, 0xac, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 30 */ > + 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0xb0, > + 0x00, 0x50, 0x90, 0x7e, 0x79, 0x70, 0x00, 0x00, /* 40 */ > + 0x00, 0xa8, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x02, 0x0d, 0x00, 0x00, 0x00, 0x00, /* 50 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 60 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x01, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 70 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 80 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0xc0, 0x00, 0x00, 0x00, /* 90 */ > + 0x0b, 0x02, 0x00, 0x18, 0x5a, 0x60, 0x00, 0x00, > + 0x00, 0x00, 0x80, 0x80, 0x08, 0x04, 0x00, 0x00, /* a0 */ > + 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x40, 0x14, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* b0 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* c0 */ > + 0x00, 0x03, 0x00, 0x00, 0x02, 0x00, 0x01, 0x04, > + 0x30, 0xff, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00, /* d0 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x01, > + 0x80, 0x75, 0x00, 0x00, 0x60, 0x00, 0x00, 0x00, /* e0 */ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x75, 0x11, 0x00, /* f0 */ > + 0x00, 0x7c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > +}; > + > +/* ADI recommanded values for proper operation. */ > +static const struct reg_default adv7511_fixed_registers[] = { > + { 0x98, 0x03 }, > + { 0x9a, 0xe0 }, > + { 0x9c, 0x30 }, > + { 0x9d, 0x61 }, > + { 0xa2, 0xa4 }, > + { 0xa3, 0xa4 }, > + { 0xe0, 0xd0 }, > + { 0xf9, 0x00 }, > + { 0x55, 0x02 }, > +}; > + > +static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder) > +{ > + return to_encoder_slave(encoder)->slave_priv; > +} > + > +static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable, > + const uint16_t *coeff, unsigned int scaling_factor) > +{ > + unsigned int i; > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), > + ADV7511_CSC_UPDATE_MODE, ADV7511_CSC_UPDATE_MODE); > + > + if (enable) { > + for (i = 0; i < 12; ++i) { > + regmap_update_bits(adv7511->regmap, > + ADV7511_REG_CSC_UPPER(i), > + 0x1f, coeff[i] >> 8); > + regmap_write(adv7511->regmap, > + ADV7511_REG_CSC_LOWER(i), > + coeff[i] & 0xff); > + } > + } You could move this in the first branch of the following if. > + > + if (enable) { > + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), > + 0xe0, 0x80 | (scaling_factor << 5)); > + } else { > + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), > + 0x80, 0x00); Could you add #defines for register fields instead of using numerical values (for values used here and below) ? > + } > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), > + ADV7511_CSC_UPDATE_MODE, 0); > +} > + > +#define ADV7511_HDMI_CFG_MODE_MASK 0x2 > +#define ADV7511_HDMI_CFG_MODE_DVI 0x0 > +#define ADV7511_HDMI_CFG_MODE_HDMI 0x2 > + > +#define ADV7511_PACKET_MEM_SPD 0 > +#define ADV7511_PACKET_MEM_MPEG 1 > +#define ADV7511_PACKET_MEM_ACP 2 > +#define ADV7511_PACKET_MEM_ISRC1 3 > +#define ADV7511_PACKET_MEM_ISRC2 4 > +#define ADV7511_PACKET_MEM_GM 5 > +#define ADV7511_PACKET_MEM_SPARE1 6 > +#define ADV7511_PACKET_MEM_SPARE2 7 > + > +#define ADV7511_PACKET_MEM_DATA_REG(x) ((x) * 0x20) > +#define ADV7511_PACKET_MEM_UPDATE_REG(x) ((x) * 0x20 + 0x1f) > +#define ADV7511_PACKET_MEM_UPDATE_ENABLE BIT(7) Should this be moved to the adv7511.h header ? > +#if 0 This should obviously be removed or used :-) > +static void adv7511_program_infoframe(struct adv7511 *adv7511, > + const void *buffer, size_t size, unsigned int reg) > +{ > + unsigned int data_reg, update_reg; > + unsigned int update_bit; > + > + switch (type) { > + case AVI: > + regmap = adv7511->regmap; > + data_reg = ADV7511_REG_AVI_INFOFRAME_VERSION; > + update_reg = ADV7511_REG_INFOFRAME_UPDATE; > + update_bit = BIT(6); > + break; > + case AUDIO: > + regmap = adv7511->regmap; > + data_reg = ADV7511_REG_AUDIO_INFOFRAME_VERSION; > + update_reg = ADV7511_REG_INFOFRAME_UPDATE; > + update_bit = BIT(5); > + break; > + case GC: > + regmap = adv7511->regmap; > + data_reg = ADV7511_REG_GC(0); > + update_reg = ADV7511_REG_INFOFRAME_UPDATE; > + update_bit = BIT(4); > + break; > + case SPD: > + regmap = adv7511->packet_memory_regmap; > + data_reg = ADV7511_PACKET_MEM_DATA_REG(ADV7511_PACKET_MEM_SPD); > + data_reg = ADV7511_PACKET_MEM_UPDATE_REG(ADV7511_PACKET_MEM_SPD); > + update_bit = ADV7511_PACKET_MEM_UPDATE_ENABLE; > + break; > + } > + > + regmap_update_bits(adv7511->regmap, update_reg, update_bit, update_bit); > + > + regmap_bulk_write(adv7511->regmap, data_reg, buffer, size); > + > + regmap_update_bits(adv7511->regmap, update_reg, update_bit, 0); > +} > + > +#endif > + > +static void adv7511_set_config(struct drm_encoder *encoder, void *c) Now we're getting to the controversial point. What bothers me about the DRM encoder slave API is that the display controller driver needs to be aware of the details of the slave encoder, as it needs to pass an encoder-specific configuration structure to the .set_config() operation. The question would thus be, what about using the Common Display Framework ? > +{ > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + struct adv7511_video_config *config = c; > + bool output_format_422, output_format_ycbcr; > + unsigned int mode; > + uint8_t infoframe[17]; > + > + if (config->hdmi_mode) { > + mode = ADV7511_HDMI_CFG_MODE_HDMI; > + > + switch (config->avi_infoframe.colorspace) { > + case HDMI_COLORSPACE_YUV444: > + output_format_422 = false; > + output_format_ycbcr = true; > + break; > + case HDMI_COLORSPACE_YUV422: > + output_format_422 = true; > + output_format_ycbcr = true; > + break; > + default: > + output_format_422 = false; > + output_format_ycbcr = false; > + break; > + } > + } else { > + mode = ADV7511_HDMI_CFG_MODE_DVI; > + output_format_422 = false; > + output_format_ycbcr = false; > + } > + > + adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); > + > + adv7511_set_colormap(adv7511, config->csc_enable, > + config->csc_coefficents, config->csc_scaling_factor); > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x81, > + (output_format_422 << 7) | output_format_ycbcr); > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > + ADV7511_HDMI_CFG_MODE_MASK, mode); > + > + hdmi_avi_infoframe_pack(&config->avi_infoframe, infoframe, > + sizeof(infoframe)); > + > + /* The AVI infoframe id is not configurable */ > + regmap_bulk_write(adv7511->regmap, ADV7511_REG_AVI_INFOFRAME_VERSION, > + infoframe + 1, sizeof(infoframe) - 1); > + > + adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); > +} > + > +static void adv7511_set_link_config(struct adv7511 *adv7511, > + const struct adv7511_link_config *config) > +{ > + enum adv7511_input_sync_pulse sync_pulse; > + > + switch (config->id) { > + case ADV7511_INPUT_ID_12_15_16BIT_RGB444_YCbCr444: > + sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE; > + break; > + default: > + sync_pulse = config->sync_pulse; > + break; > + } > + > + switch (config->id) { > + case ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_EMBEDDED_SYNC: > + case ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_EMBEDDED_SYNC: > + adv7511->embedded_sync = true; > + break; > + default: > + adv7511->embedded_sync = false; > + break; > + } > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG, > + 0xf, config->id); > + regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x7e, > + (config->input_color_depth << 4) | (config->input_style << 2)); > + regmap_write(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG2, > + (config->reverse_bitorder << 6) | > + (config->bit_justification << 3)); > + regmap_write(adv7511->regmap, ADV7511_REG_TIMING_GEN_SEQ, > + (sync_pulse << 2) | > + (config->timing_gen_seq << 1)); > + regmap_write(adv7511->regmap, 0xba, > + (config->clock_delay << 5)); > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_TMDS_CLOCK_INV, > + 0x08, config->tmds_clock_inversion << 3); > + > + adv7511->hsync_polarity = config->hsync_polarity; > + adv7511->vsync_polarity = config->vsync_polarity; > +} > + > +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet) > +{ > + if (packet & 0xff) { > + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, > + packet, 0xff); > + } > + > + if (packet & 0xff00) { > + packet >>= 8; > + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, > + packet, 0xff); > + } What about definings masks in adv7511.h ? > + > + return 0; The function never returns an error, could it be made void ? Same for the disable function below. > +} > + > +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet) > +{ > + if (packet & 0xff) { > + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, > + packet, 0x00); > + } > + > + if (packet & 0xff00) { > + packet >>= 8; > + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, > + packet, 0x00); > + } > + > + return 0; > +} > + > +static bool adv7511_register_volatile(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case ADV7511_REG_SPDIF_FREQ: > + case ADV7511_REG_CTS_AUTOMATIC1: > + case ADV7511_REG_CTS_AUTOMATIC2: > + case ADV7511_REG_VIC_DETECTED: > + case ADV7511_REG_VIC_SEND: > + case ADV7511_REG_AUX_VIC_DETECTED: > + case ADV7511_REG_STATUS: > + case ADV7511_REG_GC(1): > + case ADV7511_REG_INT(0): > + case ADV7511_REG_INT(1): > + case ADV7511_REG_PLL_STATUS: > + case ADV7511_REG_AN(0): > + case ADV7511_REG_AN(1): > + case ADV7511_REG_AN(2): > + case ADV7511_REG_AN(3): > + case ADV7511_REG_AN(4): > + case ADV7511_REG_AN(5): > + case ADV7511_REG_AN(6): > + case ADV7511_REG_AN(7): > + case ADV7511_REG_HDCP_STATUS: > + case ADV7511_REG_BCAPS: > + case ADV7511_REG_BKSV(0): > + case ADV7511_REG_BKSV(1): > + case ADV7511_REG_BKSV(2): > + case ADV7511_REG_BKSV(3): > + case ADV7511_REG_BKSV(4): > + case ADV7511_REG_DDC_STATUS: > + case ADV7511_REG_BSTATUS(0): > + case ADV7511_REG_BSTATUS(1): > + case ADV7511_REG_CHIP_ID_HIGH: > + case ADV7511_REG_CHIP_ID_LOW: > + return true; > + } > + > + return false; > +} > + > +static bool adv7511_hpd(struct adv7511 *adv7511) > +{ > + unsigned int irq0; > + int ret; > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); > + if (ret < 0) > + return false; > + > + if (irq0 & ADV7511_INT0_HDP) { > + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), ADV7511_INT0_HDP); > + return true; > + } > + > + return false; > +} > + > +static irqreturn_t adv7511_irq_handler(int irq, void *devid) > +{ > + struct adv7511 *adv7511 = devid; > + > + if (adv7511_hpd(adv7511)) > + drm_helper_hpd_irq_event(adv7511->encoder->dev); > + > + wake_up_all(&adv7511->wq); > + > + return IRQ_HANDLED; > +} > + > +static unsigned int adv7511_is_interrupt_pending(struct adv7511 *adv7511, > + unsigned int irq) > +{ > + unsigned int irq0, irq1; > + unsigned int pending; > + int ret; > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); > + if (ret < 0) > + return 0; > + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); > + if (ret < 0) > + return 0; > + > + pending = (irq1 << 8) | irq0; > + > + return pending & irq; > +} > + > +static int adv7511_wait_for_interrupt(struct adv7511 *adv7511, int irq, > + int timeout) > +{ > + unsigned int pending = 0; > + int ret; > + > + if (adv7511->i2c_main->irq) { > + ret = wait_event_interruptible_timeout(adv7511->wq, > + adv7511_is_interrupt_pending(adv7511, irq), > + msecs_to_jiffies(timeout)); > + if (ret <= 0) > + return 0; > + pending = adv7511_is_interrupt_pending(adv7511, irq); > + } else { > + if (timeout < 25) > + timeout = 25; > + do { > + pending = adv7511_is_interrupt_pending(adv7511, irq); > + if (pending) > + break; > + msleep(25); > + timeout -= 25; > + } while (timeout >= 25); > + } > + > + return pending; > +} > + > +static int adv7511_get_edid_block(void *data, unsigned char *buf, > + int block, int len) > +{ > + struct drm_encoder *encoder = data; > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + struct i2c_msg xfer[2]; > + uint8_t offset; > + int i; > + int ret; > + > + if (len > 128) > + return -EINVAL; > + > + if (adv7511->current_edid_segment != block / 2) { > + unsigned int status; > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS, &status); > + if (ret < 0) > + return ret; > + > + if (status != 2) { #define as well ? > + regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, block); > + ret = adv7511_wait_for_interrupt(adv7511, ADV7511_INT0_EDID_READY | > + ADV7511_INT1_DDC_ERROR, 200); > + > + if (!(ret & ADV7511_INT0_EDID_READY)) > + return -EIO; > + } > + > + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), > + ADV7511_INT0_EDID_READY | ADV7511_INT1_DDC_ERROR); > + > + /* Break this apart, hopefully more I2C controllers will support 64 > + * byte transfers than 256 byte transfers */ > + > + xfer[0].addr = adv7511->i2c_edid->addr; > + xfer[0].flags = 0; > + xfer[0].len = 1; > + xfer[0].buf = &offset; > + xfer[1].addr = adv7511->i2c_edid->addr; > + xfer[1].flags = I2C_M_RD; > + xfer[1].len = 64; > + xfer[1].buf = adv7511->edid_buf; > + > + offset = 0; > + > + for (i = 0; i < 4; ++i) { > + ret = i2c_transfer(adv7511->i2c_edid->adapter, xfer, ARRAY_SIZE(xfer)); > + if (ret < 0) > + return ret; > + else if (ret != 2) > + return -EIO; > + > + xfer[1].buf += 64; > + offset += 64; > + } Shouldn't you read two times 64 bytes per block, not four times ? > + > + adv7511->current_edid_segment = block / 2; > + } > + > + if (block % 2 == 0) > + memcpy(buf, adv7511->edid_buf, len); > + else > + memcpy(buf, adv7511->edid_buf + 128, len); > + > + return 0; > +} > + > +static int adv7511_get_modes(struct drm_encoder *encoder, > + struct drm_connector *connector) > +{ > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + struct edid *edid; > + unsigned int count; > + > + /* Reading the EDID only works if the device is powered */ > + if (adv7511->dpms_mode != DRM_MODE_DPMS_ON) { > + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), > + ADV7511_INT0_EDID_READY | ADV7511_INT1_DDC_ERROR); > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, 0); > + adv7511->current_edid_segment = -1; > + } > + > + edid = drm_do_get_edid(connector, adv7511_get_edid_block, encoder); > + > + if (adv7511->dpms_mode != DRM_MODE_DPMS_ON) > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); > + > + adv7511->edid = edid; > + if (!edid) > + return 0; > + > + drm_mode_connector_update_edid_property(connector, edid); > + count = drm_add_edid_modes(connector, edid); > + > + kfree(adv7511->edid); Really ? Shouldn't you then move the adv7511->edid = edid; line below ? > + > + return count; > +} > + > +struct edid *adv7511_get_edid(struct drm_encoder *encoder) > +{ > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + > + if (!adv7511->edid) > + return NULL; > + > + return kmemdup(adv7511->edid, sizeof(*adv7511->edid) + > + adv7511->edid->extensions * 128, GFP_KERNEL); > +} > +EXPORT_SYMBOL_GPL(adv7511_get_edid); Why do you need to export this function, who will use it ? > +static void adv7511_encoder_dpms(struct drm_encoder *encoder, int mode) > +{ > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + > + switch (mode) { > + case DRM_MODE_DPMS_ON: > + adv7511->current_edid_segment = -1; > + > + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), > + ADV7511_INT0_EDID_READY | ADV7511_INT1_DDC_ERROR); > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, 0); > + /* > + * Per spec it is allowed to pulse the HDP signal to indicate > + * that the EDID information has changed. Some monitors do this > + * when they wakeup from standby or are enabled. When the HDP > + * goes low the adv7511 is reset and the outputs are disabled > + * which might cause the monitor to go to standby again. To > + * avoid this we ignore the HDP pin for the first few seconds > + * after enabeling the output. s/enabeling/enabling/ > + */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, > + ADV7511_REG_POWER2_HDP_SRC_MASK, > + ADV7511_REG_POWER2_HDP_SRC_NONE); > + /* Most of the registers are reset during power down or when HPD is low */ > + regcache_sync(adv7511->regmap); > + break; > + default: > + /* TODO: setup additional power down modes */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); > + regcache_mark_dirty(adv7511->regmap); > + break; > + } > + > + adv7511->dpms_mode = mode; > +} > + > +static enum drm_connector_status adv7511_encoder_detect(struct drm_encoder *encoder, > + struct drm_connector *connector) > +{ > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + enum drm_connector_status status; > + unsigned int val; > + bool hpd; > + int ret; > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val); > + if (ret < 0) > + return connector_status_disconnected; > + > + if (val & ADV7511_STATUS_HPD) > + status = connector_status_connected; > + else > + status = connector_status_disconnected; > + > + hpd = adv7511_hpd(adv7511); > + > + /* The chip resets itself when the cable is disconnected, so in case > + * there is a pending HPD interrupt and the cable is connected there was > + * at least on transition from disconnected to connected and the chip > + * has to be reinitialized. */ > + if (status == connector_status_connected && hpd && > + adv7511->dpms_mode == DRM_MODE_DPMS_ON) { > + regcache_mark_dirty(adv7511->regmap); > + adv7511_encoder_dpms(encoder, adv7511->dpms_mode); > + adv7511_get_modes(encoder, connector); > + status = connector_status_disconnected; > + } else { > + /* Renable HDP sensing */ > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, > + ADV7511_REG_POWER2_HDP_SRC_MASK, > + ADV7511_REG_POWER2_HDP_SRC_BOTH); > + } > + > + adv7511->status = status; > + return status; > +} > + > +static void adv7511_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_display_mode *mode, > + struct drm_display_mode *adj_mode) > +{ > + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > + unsigned int low_refresh_rate; > + unsigned int hsync_polarity = 0; > + unsigned int vsync_polarity = 0; > + > + if (adv7511->embedded_sync) { > + unsigned int hsync_offset, hsync_len; > + unsigned int vsync_offset, vsync_len; > + > + hsync_offset = adj_mode->crtc_hsync_start - adj_mode->crtc_hdisplay; > + vsync_offset = adj_mode->crtc_vsync_start - adj_mode->crtc_vdisplay; > + hsync_len = adj_mode->crtc_hsync_end - adj_mode->crtc_hsync_start; > + vsync_len = adj_mode->crtc_vsync_end - adj_mode->crtc_vsync_start; > + > + /* The hardware vsync generator has a off-by-one bug */ > + vsync_offset += 1; > + > + regmap_write(adv7511->regmap, ADV7511_REG_HSYNC_PLACEMENT_MSB, > + ((hsync_offset >> 10) & 0x7) << 5); > + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(0), > + (hsync_offset >> 2) & 0xff); > + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(1), > + ((hsync_offset & 0x3) << 6) | ((hsync_len >> 4) & 0x3f)); > + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(2), > + ((hsync_len & 0xf) << 4) | ((vsync_offset >> 6) & 0xf)); > + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(3), > + ((vsync_offset & 0x3f) << 2) | ((vsync_len >> 8) & 0x3)); > + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(4), > + vsync_len & 0xff); > + > + hsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PHSYNC); > + vsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PVSYNC); > + } else { > + enum adv7511_sync_polarity mode_hsync_polarity; > + enum adv7511_sync_polarity mode_vsync_polarity; > + > + /** > + * If the input signal is always low or always high we want to > + * invert or let it passthrough depending on the polarity of the > + * current mode. > + **/ > + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC) > + mode_hsync_polarity = ADV7511_SYNC_POLARITY_LOW; > + else > + mode_hsync_polarity = ADV7511_SYNC_POLARITY_HIGH; > + > + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC) > + mode_vsync_polarity = ADV7511_SYNC_POLARITY_LOW; > + else > + mode_vsync_polarity = ADV7511_SYNC_POLARITY_HIGH; > + > + if (adv7511->hsync_polarity != mode_hsync_polarity && > + adv7511->hsync_polarity != ADV7511_SYNC_POLARITY_PASSTHROUGH) > + hsync_polarity = 1; > + > + if (adv7511->vsync_polarity != mode_vsync_polarity && > + adv7511->vsync_polarity != ADV7511_SYNC_POLARITY_PASSTHROUGH) > + vsync_polarity = 1; > + } > + > + if (mode->vrefresh <= 24000) > + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_24HZ; > + else if (mode->vrefresh <= 25000) > + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_25HZ; > + else if (mode->vrefresh <= 30000) > + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_30HZ; > + else > + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_NONE; > + > + regmap_update_bits(adv7511->regmap, 0xfb, > + 0x6, low_refresh_rate << 1); > + regmap_update_bits(adv7511->regmap, 0x17, > + 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); > + > + adv7511->f_tmds = mode->clock; > +} > + > +static struct drm_encoder_slave_funcs adv7511_encoder_funcs = { > + .set_config = adv7511_set_config, > + .dpms = adv7511_encoder_dpms, > + /* .destroy = adv7511_encoder_destroy,*/ > + .mode_set = adv7511_encoder_mode_set, > + .detect = adv7511_encoder_detect, > + .get_modes = adv7511_get_modes, > +}; > + > +static const struct regmap_config adv7511_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + > + .max_register = 0xff, > + .cache_type = REGCACHE_RBTREE, > + .reg_defaults_raw = adv7511_register_defaults, > + .num_reg_defaults_raw = ARRAY_SIZE(adv7511_register_defaults), > + > + .volatile_reg = adv7511_register_volatile, > +}; > + > +/* > + adi,input-id - > + 0x00: > + 0x01: > + 0x02: > + 0x03: > + 0x04: > + 0x05: > + adi,sync-pulse - Selects the sync pulse > + 0x00: Use the DE signal as sync pulse > + 0x01: Use the HSYNC signal as sync pulse > + 0x02: Use the VSYNC signal as sync pulse > + 0x03: No external sync pulse > + adi,bit-justification - > + 0x00: Evently > + 0x01: Right > + 0x02: Left > + adi,up-conversion - > + 0x00: zero-order up conversion > + 0x01: first-order up conversion > + adi,timing-generation-sequence - > + 0x00: Sync adjustment first, then DE generation > + 0x01: DE generation first then sync adjustment > + adi,vsync-polarity - Polarity of the vsync signal > + 0x00: Passthrough > + 0x01: Active low > + 0x02: Active high > + adi,hsync-polarity - Polarity of the hsync signal > + 0x00: Passthrough > + 0x01: Active low > + 0x02: Active high > + adi,reverse-bitorder - If set the bitorder is reveresed > + adi,tmds-clock-inversion - If set use tdms clock inversion > + adi,clock-delay - Clock delay for the video data clock > + 0x00: -1200 ps > + 0x01: -800 ps > + 0x02: -400 ps > + 0x03: no dealy > + 0x04: 400 ps > + 0x05: 800 ps > + 0x06: 1200 ps > + 0x07: 1600 ps The value should be expressed directly in ps in the DT. > + adi,input-style - Specifies the input style used > + 0x02: Use input style 1 > + 0x01: Use input style 2 > + 0x03: Use Input style 3 > + adi,input-color-depth - Selects the input format color depth > + 0x03: 8-bit per channel > + 0x01: 10-bit per channel > + 0x02: 12-bit per channel The properties related to the input format should be grouped in a single input format property, as discussed above for the input format enums. > +*/ > + > + > +static int adv7511_parse_dt(struct device_node *np, struct adv7511_link_config *config) > +{ > + int ret; > + u32 val; > + > + ret = of_property_read_u32(np, "adi,input-id", &val); > + if (ret < 0) > + return ret; > + config->id = val; > + > + ret = of_property_read_u32(np, "adi,sync-pulse", &val); > + if (ret < 0) > + config->sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE; > + else > + config->sync_pulse = val; > + > + ret = of_property_read_u32(np, "adi,bit-justification", &val); > + if (ret < 0) > + return ret; > + config->bit_justification = val; > + > + ret = of_property_read_u32(np, "adi,up-conversion", &val); > + if (ret < 0) > + config->up_conversion = ADV7511_UP_CONVERSION_ZERO_ORDER; > + else > + config->up_conversion = val; > + > + ret = of_property_read_u32(np, "adi,timing-generation-sequence", &val); > + if (ret < 0) > + return ret; > + config->timing_gen_seq = val; > + > + ret = of_property_read_u32(np, "adi,vsync-polarity", &val); > + if (ret < 0) > + return ret; > + config->vsync_polarity = val; > + > + ret = of_property_read_u32(np, "adi,hsync-polarity", &val); > + if (ret < 0) > + return ret; > + config->hsync_polarity = val; > + > + config->reverse_bitorder = of_property_read_bool(np, > + "adi,reverse-bitorder"); > + config->tmds_clock_inversion = of_property_read_bool(np, > + "adi,tmds-clock-inversion"); > + > + ret = of_property_read_u32(np, "adi,clock-delay", &val); > + if (ret) > + return ret; > + config->clock_delay = val; > + > + ret = of_property_read_u32(np, "adi,input-style", &val); > + if (ret) > + return ret; > + config->input_style = val; > + > + ret = of_property_read_u32(np, "adi,input-color-depth", &val); > + if (ret) > + return ret; > + config->input_color_depth = val; > + > + config->gpio_pd = of_get_gpio(np, 0); > + if (config->gpio_pd == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + > + return 0; > +} > + > +static const int edid_i2c_addr = 0x7e; > +static const int packet_i2c_addr = 0x70; > +static const int cec_i2c_addr = 0x78; What about #defines instead of static const ints ? > +static int adv7511_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct adv7511_link_config link_config; > + struct adv7511 *adv7511; > + unsigned int val; > + int ret; > + > + if (i2c->dev.of_node) { > + ret = adv7511_parse_dt(i2c->dev.of_node, &link_config); > + if (ret) > + return ret; > + } else { > + if (!i2c->dev.platform_data) > + return -EINVAL; > + link_config = *(struct adv7511_link_config *)i2c->dev.platform_data; > + } > + > + adv7511 = devm_kzalloc(&i2c->dev, sizeof(*adv7511), GFP_KERNEL); > + if (!adv7511) > + return -ENOMEM; > + > + adv7511->gpio_pd = link_config.gpio_pd; > + > + if (gpio_is_valid(adv7511->gpio_pd)) { > + ret = devm_gpio_request_one(&i2c->dev, adv7511->gpio_pd, > + GPIOF_OUT_INIT_HIGH, "PD"); > + if (ret) > + return ret; > + mdelay(5); msleep() or usleep_range() ? > + gpio_set_value_cansleep(adv7511->gpio_pd, 0); > + } > + > + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > + if (IS_ERR(adv7511->regmap)) > + return PTR_ERR(adv7511->regmap); > + > + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > + if (ret) > + return ret; > + dev_dbg(&i2c->dev, "Rev. %d\n", val); > + > + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > + ARRAY_SIZE(adv7511_fixed_registers)); > + if (ret) > + return ret; > + > + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); > + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, packet_i2c_addr); > + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); > + adv7511_packet_disable(adv7511, 0xffff); > + > + adv7511->i2c_main = i2c; > + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > + adv7511->i2c_packet = i2c_new_dummy(i2c->adapter, packet_i2c_addr >> 1); > + if (!adv7511->i2c_edid) > + return -ENOMEM; Wouldn't this leak i2c_packet ? > + > + if (i2c->irq) { > + ret = request_threaded_irq(i2c->irq, NULL, adv7511_irq_handler, > + IRQF_ONESHOT, dev_name(&i2c->dev), adv7511); You can use the devm_ version and get rid of the free_irq() in the remove() handler. > + if (ret) > + goto err_i2c_unregister_device; > + > + init_waitqueue_head(&adv7511->wq); > + } > + > + /* CEC is unused for now */ > + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > + ADV7511_CEC_CTRL_POWER_DOWN); > + > + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > + ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); > + > + adv7511->current_edid_segment = -1; > + > + i2c_set_clientdata(i2c, adv7511); > + adv7511_audio_init(&i2c->dev); > + > + adv7511_set_link_config(adv7511, &link_config); > + > + return 0; > + > +err_i2c_unregister_device: > + i2c_unregister_device(adv7511->i2c_edid); Shouldn't you also unregister i2c_packet ? > + > + return ret; > +} > + > +static int adv7511_remove(struct i2c_client *i2c) > +{ > + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > + > + i2c_unregister_device(adv7511->i2c_edid); Shouldn't you also unregister i2c_packet ? > + > + if (i2c->irq) > + free_irq(i2c->irq, adv7511); > + kfree(adv7511->edid); > + > + return 0; > +} > + > +static int adv7511_encoder_init(struct i2c_client *i2c, > + struct drm_device *dev, struct drm_encoder_slave *encoder) > +{ > + > + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > + > + encoder->slave_priv = adv7511; > + encoder->slave_funcs = &adv7511_encoder_funcs; > + > + adv7511->encoder = &encoder->base; > + > + return 0; > +} > + > +static const struct i2c_device_id adv7511_ids[] = { > + { "adv7511", 0 }, > + {} > +}; > + > +static struct drm_i2c_encoder_driver adv7511_driver = { > + .i2c_driver = { > + .driver = { > + .name = "adv7511", > + }, > + .id_table = adv7511_ids, > + .probe = adv7511_probe, > + .remove = adv7511_remove, > + }, > + > + .encoder_init = adv7511_encoder_init, > +}; > + > +static int __init adv7511_init(void) > +{ > + return drm_i2c_encoder_register(THIS_MODULE, &adv7511_driver); > +} > +module_init(adv7511_init); > + > +static void __exit adv7511_exit(void) > +{ > + drm_i2c_encoder_unregister(&adv7511_driver); > +} > +module_exit(adv7511_exit); > + > +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); > +MODULE_DESCRIPTION("ADV7511 HDMI transmitter driver"); > +MODULE_LICENSE("GPL");
[...] >> + >> +/** >> + * enum adv7511_input_color_depth - Selects the input format color depth >> + * @ADV7511_INPUT_COLOR_DEPTH_8BIT: Input format color depth is 8 bits per > channel >> + * @ADV7511_INPUT_COLOR_DEPTH_10BIT: Input format color dpeth is 10 bits > per channel >> + * @ADV7511_INPUT_COLOR_DEPTH_12BIT: Input format color depth is 12 bits > per channel >> + **/ >> +enum adv7511_input_color_depth { >> + ADV7511_INPUT_COLOR_DEPTH_8BIT = 3, >> + ADV7511_INPUT_COLOR_DEPTH_10BIT = 1, >> + ADV7511_INPUT_COLOR_DEPTH_12BIT = 2, >> +}; > > Those enums describe the video format at the chip input. This can be > configured statically from platform data or DT, but some platforms might need > to setup formats dynamically at runtime. For instance the ADV7511 could be > driven by a mux with two inputs using different formats. > > For these reasons I would combine all those enums in a single one that lists > all possible input formats. The formats should be standardized and moved to a > separate header file. Get and set format operations will be needed (this is > something CDF will address :-)). Since most these settings are orthogonal to each other putting them in one enum gives you 3 * 3 * 6 * 3 = 162 entries. These enums configure to which pins of the ADV7511 what signal is connected. Standardizing this would require that other chips use the same layouts for connecting the pins. [...] >> + >> +/** >> + * enum adv7511_up_conversion - Selects the upscaling conversion method >> + * @ADV7511_UP_CONVERSION_ZERO_ORDER: Use zero order up conversion >> + * @ADV7511_UP_CONVERSION_FIRST_ORDER: Use first order up conversion >> + * >> + * This used when converting from a 4:2:2 format to a 4:4:4 format. >> + **/ >> +enum adv7511_up_conversion { >> + ADV7511_UP_CONVERSION_ZERO_ORDER = 0, >> + ADV7511_UP_CONVERSION_FIRST_ORDER = 1, >> +}; > > How is the upscaling conversion method supposed to be selected ? What does it > depend on ? > It's probably up to the system designer to say which method yields better results for their system. [...] >> +/** >> + * struct adv7511_video_config - Describes adv7511 hardware configuration >> + * @csc_enable: Whether to enable color space conversion >> + * @csc_scaling_factor: Color space conversion scaling factor >> + * @csc_coefficents: Color space conversion coefficents >> + * @hdmi_mode: Whether to use HDMI or DVI output mode >> + * @avi_infoframe: HDMI infoframe >> + **/ >> +struct adv7511_video_config { >> + bool csc_enable; > > Shouldn't this be automatically computed based on the input and output formats > ? If the driver knew the input and output colorspaces and had coefficient tables for all possible combinations built in that could be done. This is maybe something that could be done in the framework. > >> + enum adv7511_csc_scaling csc_scaling_factor; >> + const uint16_t *csc_coefficents; > > Do the coefficients need to be configured freely, or could presets do ? > >> + bool hdmi_mode; >> + struct hdmi_avi_infoframe avi_infoframe; >> +}; [...] >> +static void adv7511_set_config(struct drm_encoder *encoder, void *c) > > Now we're getting to the controversial point. > > What bothers me about the DRM encoder slave API is that the display controller > driver needs to be aware of the details of the slave encoder, as it needs to > pass an encoder-specific configuration structure to the .set_config() > operation. The question would thus be, what about using the Common Display > Framework ? Well, as I said I'd prefer using the CDF for this driver. But even then the display controller driver might need to know about the details of the encoder. I think we talked about this during the FOSDEM meeting. The graphics fabric on the board can easily get complex enough to require a board custom driver, similar to what we have in ASoC. I think this drm_bridge patchset[1] tries to address a similar issue. [1] http://lists.freedesktop.org/archives/dri-devel/2013-August/043237.html [...] >> + >> + for (i = 0; i < 4; ++i) { >> + ret = i2c_transfer(adv7511->i2c_edid->adapter, xfer, > ARRAY_SIZE(xfer)); >> + if (ret < 0) >> + return ret; >> + else if (ret != 2) >> + return -EIO; >> + >> + xfer[1].buf += 64; >> + offset += 64; >> + } > > Shouldn't you read two times 64 bytes per block, not four times ? The controller on the ADV7511 always reads two blocks at once, so it is 256 bytes. > >> + >> + adv7511->current_edid_segment = block / 2; >> + } >> + >> + if (block % 2 == 0) >> + memcpy(buf, adv7511->edid_buf, len); >> + else >> + memcpy(buf, adv7511->edid_buf + 128, len); >> + >> + return 0; >> +} >> + [...] >> + >> +struct edid *adv7511_get_edid(struct drm_encoder *encoder) >> +{ >> + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); >> + >> + if (!adv7511->edid) >> + return NULL; >> + >> + return kmemdup(adv7511->edid, sizeof(*adv7511->edid) + >> + adv7511->edid->extensions * 128, GFP_KERNEL); >> +} >> +EXPORT_SYMBOL_GPL(adv7511_get_edid); > > Why do you need to export this function, who will use it ? The drm driver using the encoder that wants to know the EDID. E.g. to know if the monitor supports HDMI or not. But exporting this function is really just a quick hack to compensate for the removal of 'raw_edid', this probably wants proper abstraction in the framework. [...] >> +/* >> + adi,input-id - >> + 0x00: >> + 0x01: >> + 0x02: >> + 0x03: >> + 0x04: >> + 0x05: >> + adi,sync-pulse - Selects the sync pulse >> + 0x00: Use the DE signal as sync pulse >> + 0x01: Use the HSYNC signal as sync pulse >> + 0x02: Use the VSYNC signal as sync pulse >> + 0x03: No external sync pulse >> + adi,bit-justification - >> + 0x00: Evently >> + 0x01: Right >> + 0x02: Left >> + adi,up-conversion - >> + 0x00: zero-order up conversion >> + 0x01: first-order up conversion >> + adi,timing-generation-sequence - >> + 0x00: Sync adjustment first, then DE generation >> + 0x01: DE generation first then sync adjustment >> + adi,vsync-polarity - Polarity of the vsync signal >> + 0x00: Passthrough >> + 0x01: Active low >> + 0x02: Active high >> + adi,hsync-polarity - Polarity of the hsync signal >> + 0x00: Passthrough >> + 0x01: Active low >> + 0x02: Active high >> + adi,reverse-bitorder - If set the bitorder is reveresed >> + adi,tmds-clock-inversion - If set use tdms clock inversion >> + adi,clock-delay - Clock delay for the video data clock >> + 0x00: -1200 ps >> + 0x01: -800 ps >> + 0x02: -400 ps >> + 0x03: no dealy >> + 0x04: 400 ps >> + 0x05: 800 ps >> + 0x06: 1200 ps >> + 0x07: 1600 ps > > The value should be expressed directly in ps in the DT. DT doesn't allow negative values. Thanks for the review, - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars-Peter, On Wednesday 04 September 2013 13:34:30 Lars-Peter Clausen wrote: > [...] > > >> + > >> +/** > >> + * enum adv7511_input_color_depth - Selects the input format color depth > >> + * @ADV7511_INPUT_COLOR_DEPTH_8BIT: Input format color depth is 8 bits > >> per channel > >> + * @ADV7511_INPUT_COLOR_DEPTH_10BIT: Input format color dpeth is 10 bits > >> per channel > >> + * @ADV7511_INPUT_COLOR_DEPTH_12BIT: Input format color depth is 12 bits > >> per channel > >> + **/ > >> +enum adv7511_input_color_depth { > >> + ADV7511_INPUT_COLOR_DEPTH_8BIT = 3, > >> + ADV7511_INPUT_COLOR_DEPTH_10BIT = 1, > >> + ADV7511_INPUT_COLOR_DEPTH_12BIT = 2, > >> +}; > > > > Those enums describe the video format at the chip input. This can be > > configured statically from platform data or DT, but some platforms might > > need to setup formats dynamically at runtime. For instance the ADV7511 > > could be driven by a mux with two inputs using different formats. > > > > For these reasons I would combine all those enums in a single one that > > lists all possible input formats. The formats should be standardized and > > moved to a separate header file. Get and set format operations will be > > needed (this is something CDF will address :-)). > > Since most these settings are orthogonal to each other putting them in one > enum gives you 3 * 3 * 6 * 3 = 162 entries. These enums configure to which > pins of the ADV7511 what signal is connected. Standardizing this would > require that other chips use the same layouts for connecting the pins. The problem is, this information needs to be shared between devices. Formats on the bus could very well be dynamic, the ADV7511 video source could be able to generate multiple formats that would be runtime configurable. To support this, platform data and DT bindings should describe how signals are connected (which would thus filter the list of supported formats), and a runtime API will be needed to get and set formats. The formats thus need to be standardized We had a similar issue in V4L2 and have introduced a media bus format enumeration to solve it (see include/uapi/linux/v4l2-mediabus.h). I believe something similar is needed here. Of course this won't solve the issue of how to select a format on the bus when both endpoints support multiple formats. This should probably be selected by userspace somehow, but we're missing an API to do so at the moment. A default value would thus need to be computed somehow, and possibly given by platform data and DT bindings (although it doesn't describe the hardware as such, and should thus not be part of the DT bindings). > >> +/** > >> + * enum adv7511_up_conversion - Selects the upscaling conversion method > >> + * @ADV7511_UP_CONVERSION_ZERO_ORDER: Use zero order up conversion > >> + * @ADV7511_UP_CONVERSION_FIRST_ORDER: Use first order up conversion > >> + * > >> + * This used when converting from a 4:2:2 format to a 4:4:4 format. > >> + **/ > >> +enum adv7511_up_conversion { > >> + ADV7511_UP_CONVERSION_ZERO_ORDER = 0, > >> + ADV7511_UP_CONVERSION_FIRST_ORDER = 1, > >> +}; > > > > How is the upscaling conversion method supposed to be selected ? What does > > it depend on ? > > It's probably up to the system designer to say which method yields better > results for their system. And what would a system designer base his/her decision on ? :-) > >> +/** > >> + * struct adv7511_video_config - Describes adv7511 hardware > >> configuration > >> + * @csc_enable: Whether to enable color space conversion > >> + * @csc_scaling_factor: Color space conversion scaling factor > >> + * @csc_coefficents: Color space conversion coefficents > >> + * @hdmi_mode: Whether to use HDMI or DVI output mode > >> + * @avi_infoframe: HDMI infoframe > >> + **/ > >> +struct adv7511_video_config { > >> + bool csc_enable; > > > > Shouldn't this be automatically computed based on the input and output > > formats ? > > If the driver knew the input and output colorspaces and had coefficient > tables for all possible combinations built in that could be done. This is > maybe something that could be done in the framework. > > >> + enum adv7511_csc_scaling csc_scaling_factor; > >> + const uint16_t *csc_coefficents; > > > > Do the coefficients need to be configured freely, or could presets do ? > > > >> + bool hdmi_mode; > >> + struct hdmi_avi_infoframe avi_infoframe; > >> +}; > > [...] > > >> +static void adv7511_set_config(struct drm_encoder *encoder, void *c) > > > > Now we're getting to the controversial point. > > > > What bothers me about the DRM encoder slave API is that the display > > controller driver needs to be aware of the details of the slave encoder, > > as it needs to pass an encoder-specific configuration structure to the > > .set_config() operation. The question would thus be, what about using the > > Common Display Framework ? > > Well, as I said I'd prefer using the CDF for this driver. But even then the > display controller driver might need to know about the details of the > encoder. I think we talked about this during the FOSDEM meeting. The > graphics fabric on the board can easily get complex enough to require a > board custom driver, similar to what we have in ASoC. I think this > drm_bridge patchset[1] tries to address a similar issue. > > [1] http://lists.freedesktop.org/archives/dri-devel/2013-August/043237.html > [...] > > >> + > >> + for (i = 0; i < 4; ++i) { > >> + ret = i2c_transfer(adv7511->i2c_edid->adapter, xfer, > >> ARRAY_SIZE(xfer)); > >> + if (ret < 0) > >> + return ret; > >> + else if (ret != 2) > >> + return -EIO; > >> + > >> + xfer[1].buf += 64; > >> + offset += 64; > >> + } > > > > Shouldn't you read two times 64 bytes per block, not four times ? > > The controller on the ADV7511 always reads two blocks at once, so it is 256 > bytes. But this function return 128 bytes to the called. Can't you optimize this by reading 128 bytes only from the ADV7511 instead of throwing away 128 bytes every time ? > >> + > >> + adv7511->current_edid_segment = block / 2; > >> + } > >> + > >> + if (block % 2 == 0) > >> + memcpy(buf, adv7511->edid_buf, len); > >> + else > >> + memcpy(buf, adv7511->edid_buf + 128, len); > >> + > >> + return 0; > >> +} > >> + > > [...] > > >> + > >> +struct edid *adv7511_get_edid(struct drm_encoder *encoder) > >> +{ > >> + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); > >> + > >> + if (!adv7511->edid) > >> + return NULL; > >> + > >> + return kmemdup(adv7511->edid, sizeof(*adv7511->edid) + > >> + adv7511->edid->extensions * 128, GFP_KERNEL); > >> +} > >> +EXPORT_SYMBOL_GPL(adv7511_get_edid); > > > > Why do you need to export this function, who will use it ? > > The drm driver using the encoder that wants to know the EDID. E.g. to know > if the monitor supports HDMI or not. But exporting this function is really > just a quick hack to compensate for the removal of 'raw_edid', this probably > wants proper abstraction in the framework. Yes, we do want a proper abstraction :-) > >> +/* > >> + adi,input-id - > >> + 0x00: > >> + 0x01: > >> + 0x02: > >> + 0x03: > >> + 0x04: > >> + 0x05: > >> + adi,sync-pulse - Selects the sync pulse > >> + 0x00: Use the DE signal as sync pulse > >> + 0x01: Use the HSYNC signal as sync pulse > >> + 0x02: Use the VSYNC signal as sync pulse > >> + 0x03: No external sync pulse > >> + adi,bit-justification - > >> + 0x00: Evently > >> + 0x01: Right > >> + 0x02: Left > >> + adi,up-conversion - > >> + 0x00: zero-order up conversion > >> + 0x01: first-order up conversion > >> + adi,timing-generation-sequence - > >> + 0x00: Sync adjustment first, then DE generation > >> + 0x01: DE generation first then sync adjustment > >> + adi,vsync-polarity - Polarity of the vsync signal > >> + 0x00: Passthrough > >> + 0x01: Active low > >> + 0x02: Active high > >> + adi,hsync-polarity - Polarity of the hsync signal > >> + 0x00: Passthrough > >> + 0x01: Active low > >> + 0x02: Active high > >> + adi,reverse-bitorder - If set the bitorder is reveresed > >> + adi,tmds-clock-inversion - If set use tdms clock inversion > >> + adi,clock-delay - Clock delay for the video data clock > >> + 0x00: -1200 ps > >> + 0x01: -800 ps > >> + 0x02: -400 ps > >> + 0x03: no dealy > >> + 0x04: 400 ps > >> + 0x05: 800 ps > >> + 0x06: 1200 ps > >> + 0x07: 1600 ps > > > > The value should be expressed directly in ps in the DT. > > DT doesn't allow negative values. Doesn't it ? DT carries values a packed bits, but doesn't really specify how the value is to be interpreted. That's the job of the DT bindings. We could just tell that the clock-delay should be a 32-bit 2-complement signed value.
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index a7c54c8..7cd1c29 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -200,6 +200,12 @@ config DRM_SAVAGE Choose this option if you have a Savage3D/4/SuperSavage/Pro/Twister chipset. If M is selected the module will be called savage. +config DRM_ENCODER_ADV7511 + tristate "AV7511 encoder" + depends on DRM && I2C + select REGMAP_I2C + select HDMI + source "drivers/gpu/drm/exynos/Kconfig" source "drivers/gpu/drm/vmwgfx/Kconfig" diff --git a/drivers/gpu/drm/i2c/Makefile b/drivers/gpu/drm/i2c/Makefile index 43aa33b..ef3a76a 100644 --- a/drivers/gpu/drm/i2c/Makefile +++ b/drivers/gpu/drm/i2c/Makefile @@ -6,5 +6,8 @@ obj-$(CONFIG_DRM_I2C_CH7006) += ch7006.o sil164-y := sil164_drv.o obj-$(CONFIG_DRM_I2C_SIL164) += sil164.o +adv7511-y := adv7511_core.o adv7511_audio.o +obj-$(CONFIG_DRM_ENCODER_ADV7511) += adv7511.o + tda998x-y := tda998x_drv.o obj-$(CONFIG_DRM_I2C_NXP_TDA998X) += tda998x.o diff --git a/drivers/gpu/drm/i2c/adv7511.h b/drivers/gpu/drm/i2c/adv7511.h new file mode 100644 index 0000000..55f8203 --- /dev/null +++ b/drivers/gpu/drm/i2c/adv7511.h @@ -0,0 +1,455 @@ +/** + * Analog Devices ADV7511 HDMI transmitter driver + * + * Copyright 2012 Analog Devices Inc. + * + * Licensed under the GPL-2. + */ + +#ifndef __ADV7511_H__ +#define __ADV7511_H__ + +#include <linux/hdmi.h> + +#define ADV7511_REG_CHIP_REVISION 0x00 +#define ADV7511_REG_N0 0x01 +#define ADV7511_REG_N1 0x02 +#define ADV7511_REG_N2 0x03 +#define ADV7511_REG_SPDIF_FREQ 0x04 +#define ADV7511_REG_CTS_AUTOMATIC1 0x05 +#define ADV7511_REG_CTS_AUTOMATIC2 0x06 +#define ADV7511_REG_CTS_MANUAL0 0x07 +#define ADV7511_REG_CTS_MANUAL1 0x08 +#define ADV7511_REG_CTS_MANUAL2 0x09 +#define ADV7511_REG_AUDIO_SOURCE 0x0a +#define ADV7511_REG_AUDIO_CONFIG 0x0b +#define ADV7511_REG_I2S_CONFIG 0x0c +#define ADV7511_REG_I2S_WIDTH 0x0d +#define ADV7511_REG_AUDIO_SUB_SRC0 0x0e +#define ADV7511_REG_AUDIO_SUB_SRC1 0x0f +#define ADV7511_REG_AUDIO_SUB_SRC2 0x10 +#define ADV7511_REG_AUDIO_SUB_SRC3 0x11 +#define ADV7511_REG_AUDIO_CFG1 0x12 +#define ADV7511_REG_AUDIO_CFG2 0x13 +#define ADV7511_REG_AUDIO_CFG3 0x14 +#define ADV7511_REG_I2C_FREQ_ID_CFG 0x15 +#define ADV7511_REG_VIDEO_INPUT_CFG1 0x16 +#define ADV7511_REG_CSC_UPPER(x) (0x18 + (x) * 2) +#define ADV7511_REG_CSC_LOWER(x) (0x19 + (x) * 2) +#define ADV7511_REG_SYNC_DECODER(x) (0x30 + (x)) +#define ADV7511_REG_DE_GENERATOR (0x35 + (x)) +#define ADV7511_REG_PIXEL_REPETITION 0x3b +#define ADV7511_REG_VIC_MANUAL 0x3c +#define ADV7511_REG_VIC_SEND 0x3d +#define ADV7511_REG_VIC_DETECTED 0x3e +#define ADV7511_REG_AUX_VIC_DETECTED 0x3f +#define ADV7511_REG_PACKET_ENABLE0 0x40 +#define ADV7511_REG_POWER 0x41 +#define ADV7511_REG_STATUS 0x42 +#define ADV7511_REG_EDID_I2C_ADDR 0x43 +#define ADV7511_REG_PACKET_ENABLE1 0x44 +#define ADV7511_REG_PACKET_I2C_ADDR 0x45 +#define ADV7511_REG_DSD_ENABLE 0x46 +#define ADV7511_REG_VIDEO_INPUT_CFG2 0x48 +#define ADV7511_REG_INFOFRAME_UPDATE 0x4a +#define ADV7511_REG_GC(x) (0x4b + (x)) /* 0x4b - 0x51 */ +#define ADV7511_REG_AVI_INFOFRAME_VERSION 0x52 +#define ADV7511_REG_AVI_INFOFRAME_LENGTH 0x53 +#define ADV7511_REG_AVI_INFOFRAME_CHECKSUM 0x54 +#define ADV7511_REG_AVI_INFOFRAME(x) (0x55 + (x)) /* 0x55 - 0x6f */ +#define ADV7511_REG_AUDIO_INFOFRAME_VERSION 0x70 +#define ADV7511_REG_AUDIO_INFOFRAME_LENGTH 0x71 +#define ADV7511_REG_AUDIO_INFOFRAME_CHECKSUM 0x72 +#define ADV7511_REG_AUDIO_INFOFRAME(x) (0x73 + (x)) /* 0x73 - 0x7c */ +#define ADV7511_REG_INT_ENABLE(x) (0x94 + (x)) +#define ADV7511_REG_INT(x) (0x96 + (x)) +#define ADV7511_REG_INPUT_CLK_DIV 0x9d +#define ADV7511_REG_PLL_STATUS 0x9e +#define ADV7511_REG_HDMI_POWER 0xa1 +#define ADV7511_REG_HDCP_HDMI_CFG 0xaf +#define ADV7511_REG_AN(x) (0xb0 + (x)) /* 0xb0 - 0xb7 */ +#define ADV7511_REG_HDCP_STATUS 0xb8 +#define ADV7511_REG_BCAPS 0xbe +#define ADV7511_REG_BKSV(x) (0xc0 + (x)) /* 0xc0 - 0xc3 */ +#define ADV7511_REG_EDID_SEGMENT 0xc4 +#define ADV7511_REG_DDC_STATUS 0xc8 +#define ADV7511_REG_EDID_READ_CTRL 0xc9 +#define ADV7511_REG_BSTATUS(x) (0xca + (x)) /* 0xca - 0xcb */ +#define ADV7511_REG_TIMING_GEN_SEQ 0xd0 +#define ADV7511_REG_POWER2 0xd6 +#define ADV7511_REG_HSYNC_PLACEMENT_MSB 0xfa + +#define ADV7511_REG_SYNC_ADJUSTMENT(x) (0xd7 + (x)) /* 0xd7 - 0xdc */ +#define ADV7511_REG_TMDS_CLOCK_INV 0xde +#define ADV7511_REG_ARC_CTRL 0xdf +#define ADV7511_REG_CEC_I2C_ADDR 0xe1 +#define ADV7511_REG_CEC_CTRL 0xe2 +#define ADV7511_REG_CHIP_ID_HIGH 0xf5 +#define ADV7511_REG_CHIP_ID_LOW 0xf6 + +#define ADV7511_CSC_ENABLE BIT(7) +#define ADV7511_CSC_UPDATE_MODE BIT(5) + +#define ADV7511_INT0_HDP BIT(7) +#define ADV7511_INT0_VSYNC BIT(5) +#define ADV7511_INT0_AUDIO_FIFO_FULL BIT(4) +#define ADV7511_INT0_EDID_READY BIT(2) +#define ADV7511_INT0_HDCP_AUTHENTICATED BIT(1) + +#define ADV7511_INT1_DDC_ERROR BIT(7) +#define ADV7511_INT1_BKSV BIT(6) +#define ADV7511_INT1_CEC_TX_READY BIT(5) +#define ADV7511_INT1_CEC_TX_ARBIT_LOST BIT(4) +#define ADV7511_INT1_CEC_TX_RETRY_TIMEOUT BIT(3) +#define ADV7511_INT1_CEC_RX_READY3 BIT(2) +#define ADV7511_INT1_CEC_RX_READY2 BIT(1) +#define ADV7511_INT1_CEC_RX_READY1 BIT(0) + +#define ADV7511_ARC_CTRL_POWER_DOWN BIT(0) + +#define ADV7511_CEC_CTRL_POWER_DOWN BIT(0) + +#define ADV7511_POWER_POWER_DOWN BIT(6) + +#define ADV7511_AUDIO_SELECT_I2C 0x0 +#define ADV7511_AUDIO_SELECT_SPDIF 0x1 +#define ADV7511_AUDIO_SELECT_DSD 0x2 +#define ADV7511_AUDIO_SELECT_HBR 0x3 +#define ADV7511_AUDIO_SELECT_DST 0x4 + +#define ADV7511_I2S_SAMPLE_LEN_16 0x2 +#define ADV7511_I2S_SAMPLE_LEN_20 0x3 +#define ADV7511_I2S_SAMPLE_LEN_18 0x4 +#define ADV7511_I2S_SAMPLE_LEN_22 0x5 +#define ADV7511_I2S_SAMPLE_LEN_19 0x8 +#define ADV7511_I2S_SAMPLE_LEN_23 0x9 +#define ADV7511_I2S_SAMPLE_LEN_24 0xb +#define ADV7511_I2S_SAMPLE_LEN_17 0xc +#define ADV7511_I2S_SAMPLE_LEN_21 0xd + +#define ADV7511_SAMPLE_FREQ_44100 0x0 +#define ADV7511_SAMPLE_FREQ_48000 0x2 +#define ADV7511_SAMPLE_FREQ_32000 0x3 +#define ADV7511_SAMPLE_FREQ_88200 0x8 +#define ADV7511_SAMPLE_FREQ_96000 0xa +#define ADV7511_SAMPLE_FREQ_176400 0xc +#define ADV7511_SAMPLE_FREQ_192000 0xe + +#define ADV7511_STATUS_POWER_DOWN_POLARITY BIT(7) +#define ADV7511_STATUS_HPD BIT(6) +#define ADV7511_STATUS_MONITOR_SENSE BIT(5) +#define ADV7511_STATUS_I2S_32BIT_MODE BIT(3) + +#define ADV7511_PACKET_ENABLE_N_CTS BIT(8+6) +#define ADV7511_PACKET_ENABLE_AUDIO_SAMPLE BIT(8+5) +#define ADV7511_PACKET_ENABLE_AVI_INFOFRAME BIT(8+4) +#define ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME BIT(8+3) +#define ADV7511_PACKET_ENABLE_GC BIT(7) +#define ADV7511_PACKET_ENABLE_SPD BIT(6) +#define ADV7511_PACKET_ENABLE_MPEG BIT(5) +#define ADV7511_PACKET_ENABLE_ACP BIT(4) +#define ADV7511_PACKET_ENABLE_ISRC BIT(3) +#define ADV7511_PACKET_ENABLE_GM BIT(2) +#define ADV7511_PACKET_ENABLE_SPARE2 BIT(1) +#define ADV7511_PACKET_ENABLE_SPARE1 BIT(0) + +#define ADV7511_REG_POWER2_HDP_SRC_MASK 0xc0 +#define ADV7511_REG_POWER2_HDP_SRC_BOTH 0x00 +#define ADV7511_REG_POWER2_HDP_SRC_HDP 0x40 +#define ADV7511_REG_POWER2_HDP_SRC_CEC 0x80 +#define ADV7511_REG_POWER2_HDP_SRC_NONE 0xc0 +#define ADV7511_REG_POWER2_TDMS_ENABLE BIT(4) +#define ADV7511_REG_POWER2_GATE_INPUT_CLK BIT(0) + +#define ADV7511_LOW_REFRESH_RATE_NONE 0x0 +#define ADV7511_LOW_REFRESH_RATE_24HZ 0x1 +#define ADV7511_LOW_REFRESH_RATE_25HZ 0x2 +#define ADV7511_LOW_REFRESH_RATE_30HZ 0x3 + +#define ADV7511_AUDIO_CFG3_LEN_MASK 0x0f +#define ADV7511_I2C_FREQ_ID_CFG_RATE_MASK 0xf0 + +#define ADV7511_AUDIO_SOURCE_I2S 0 +#define ADV7511_AUDIO_SOURCE_SPDIF 1 + +#define ADV7511_I2S_FORMAT_I2S 0 +#define ADV7511_I2S_FORMAT_RIGHT_J 1 +#define ADV7511_I2S_FORMAT_LEFT_J 2 + +#define ADV7511_PACKET(p, x) ((p) * 0x20 + (x)) +#define ADV7511_PACKET_SDP(x) ADV7511_PACKET(0, x) +#define ADV7511_PACKET_MPEG(x) ADV7511_PACKET(1, x) +#define ADV7511_PACKET_ACP(x) ADV7511_PACKET(2, x) +#define ADV7511_PACKET_ISRC1(x) ADV7511_PACKET(3, x) +#define ADV7511_PACKET_ISRC2(x) ADV7511_PACKET(4, x) +#define ADV7511_PACKET_GM(x) ADV7511_PACKET(5, x) +#define ADV7511_PACKET_SPARE(x) ADV7511_PACKET(6, x) + +#include <drm/drmP.h> + +struct i2c_client; +struct regmap; +struct adv7511; + +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet); +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet); + +int adv7511_audio_init(struct device *dev); +void adv7511_audio_exit(struct device *dev); + +/** + * enum adv7511_input_style - Selects the input format style + * ADV7511_INPUT_STYLE1: Use input style 1 + * ADV7511_INPUT_STYLE2: Use input style 2 + * ADV7511_INPUT_STYLE3: Use input style 3 + **/ +enum adv7511_input_style { + ADV7511_INPUT_STYLE1 = 2, + ADV7511_INPUT_STYLE2 = 1, + ADV7511_INPUT_STYLE3 = 3, +}; + +/** + * enum adv7511_input_id - Selects the input format id + * @ADV7511_INPUT_ID_24BIT_RGB444_YCbCr444: Input pixel format is 24-bit 444 RGB + * or 444 YCbCR with separate syncs + * @ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_SEPARATE_SYNC: + * @ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_EMBEDDED_SYNC: + * @ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_SEPARATE_SYNC: + * @ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_EMBEDDED_SYNC: + * @ADV7511_INPUT_ID_12_15_16BIT_RGB444_YCbCr444: + **/ +enum adv7511_input_id { + ADV7511_INPUT_ID_24BIT_RGB444_YCbCr444 = 0, + ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_SEPARATE_SYNC = 1, + ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_EMBEDDED_SYNC = 2, + ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_SEPARATE_SYNC = 3, + ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_EMBEDDED_SYNC = 4, + ADV7511_INPUT_ID_12_15_16BIT_RGB444_YCbCr444 = 5, +}; + +/** + * enum adv7511_input_bit_justifiction - Selects the input format bit justifiction + * ADV7511_INPUT_BIT_JUSTIFICATION_EVENLY: Input bits are evenly distributed + * ADV7511_INPUT_BIT_JUSTIFICATION_RIGHT: Input bit signals have right justification + * ADV7511_INPUT_BIT_JUSTIFICATION_LEFT: Input bit signals have left justification + **/ +enum adv7511_input_bit_justifiction { + ADV7511_INPUT_BIT_JUSTIFICATION_EVENLY = 0, + ADV7511_INPUT_BIT_JUSTIFICATION_RIGHT = 1, + ADV7511_INPUT_BIT_JUSTIFICATION_LEFT = 2, +}; + +/** + * enum adv7511_input_color_depth - Selects the input format color depth + * @ADV7511_INPUT_COLOR_DEPTH_8BIT: Input format color depth is 8 bits per channel + * @ADV7511_INPUT_COLOR_DEPTH_10BIT: Input format color dpeth is 10 bits per channel + * @ADV7511_INPUT_COLOR_DEPTH_12BIT: Input format color depth is 12 bits per channel + **/ +enum adv7511_input_color_depth { + ADV7511_INPUT_COLOR_DEPTH_8BIT = 3, + ADV7511_INPUT_COLOR_DEPTH_10BIT = 1, + ADV7511_INPUT_COLOR_DEPTH_12BIT = 2, +}; + +/** + * enum adv7511_input_sync_pulse - Selects the sync pulse + * @ADV7511_INPUT_SYNC_PULSE_DE: Use the DE signal as sync pulse + * @ADV7511_INPUT_SYNC_PULSE_HSYNC: Use the HSYNC signal as sync pulse + * @ADV7511_INPUT_SYNC_PULSE_VSYNC: Use the VSYNC signal as sync pulse + * @ADV7511_INPUT_SYNC_PULSE_NONE: No external sync pulse signal + **/ +enum adv7511_input_sync_pulse { + ADV7511_INPUT_SYNC_PULSE_DE = 0, + ADV7511_INPUT_SYNC_PULSE_HSYNC = 1, + ADV7511_INPUT_SYNC_PULSE_VSYNC = 2, + ADV7511_INPUT_SYNC_PULSE_NONE = 3, +}; + +/** + * enum adv7511_input_clock_delay - Delay for the video data input clock + * @ADV7511_INPUT_CLOCK_DELAY_MINUS_1200PS: -1200 pico seconds delay + * @ADV7511_INPUT_CLOCK_DELAY_MINUS_800PS: -800 pico seconds delay + * @ADV7511_INPUT_CLOCK_DELAY_MINUS_400PS: -400 pico seconds delay + * @ADV7511_INPUT_CLOCK_DELAY_NONE: No delay + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_400PS: 400 pico seconds delay + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_800PS: 800 pico seconds delay + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_1200PS: 1200 pico seconds delay + * @ADV7511_INPUT_CLOCK_DELAY_PLUS_1600PS: 1600 pico seconds delay + **/ +enum adv7511_input_clock_delay { + ADV7511_INPUT_CLOCK_DELAY_MINUS_1200PS = 0, + ADV7511_INPUT_CLOCK_DELAY_MINUS_800PS = 1, + ADV7511_INPUT_CLOCK_DELAY_MINUS_400PS = 2, + ADV7511_INPUT_CLOCK_DELAY_NONE = 3, + ADV7511_INPUT_CLOCK_DELAY_PLUS_400PS = 4, + ADV7511_INPUT_CLOCK_DELAY_PLUS_800PS = 5, + ADV7511_INPUT_CLOCK_DELAY_PLUS_1200PS = 6, + ADV7511_INPUT_CLOCK_DELAY_PLUS_1600PS = 7, +}; + +/** + * enum adv7511_sync_polarity - Polarity for the input sync signals + * ADV7511_SYNC_POLARITY_PASSTHROUGH: Sync polarity matches that of the currently + * configured mode. + * ADV7511_SYNC_POLARITY_LOW: Sync polarity is low + * ADV7511_SYNC_POLARITY_HIGH: Sync polarity is high + * + * If the polarity is set to either ADV7511_SYNC_POLARITY_LOW or + * ADV7511_SYNC_POLARITY_HIGH the ADV7511 will internally invert the signal if + * it is required to match the sync polarity setting for the currently selected + * mode. If the polarity is set to ADV7511_SYNC_POLARITY_PASSTHROUGH, the ADV7511 + * will route the signal unchanged, this is useful if the upstream graphics core + * will already generate the sync singals with the correct polarity. + **/ +enum adv7511_sync_polarity { + ADV7511_SYNC_POLARITY_PASSTHROUGH, + ADV7511_SYNC_POLARITY_LOW, + ADV7511_SYNC_POLARITY_HIGH, +}; + +/** + * enum adv7511_timing_gen_seq - Selects the order in which timing adjustments are performed + * @ADV7511_TIMING_GEN_SEQ_SYN_ADJ_FIRST: Sync adjustment first, then DE generation + * @ADV7511_TIMING_GEN_SEQ_DE_GEN_FIRST: DE generation first, then sync adjustment + * + * This setting is only relevant if both DE generation and sync adjustment are + * active. + **/ +enum adv7511_timing_gen_seq { + ADV7511_TIMING_GEN_SEQ_SYN_ADJ_FIRST = 0, + ADV7511_TIMING_GEN_SEQ_DE_GEN_FIRST = 1, +}; + +/** + * enum adv7511_up_conversion - Selects the upscaling conversion method + * @ADV7511_UP_CONVERSION_ZERO_ORDER: Use zero order up conversion + * @ADV7511_UP_CONVERSION_FIRST_ORDER: Use first order up conversion + * + * This used when converting from a 4:2:2 format to a 4:4:4 format. + **/ +enum adv7511_up_conversion { + ADV7511_UP_CONVERSION_ZERO_ORDER = 0, + ADV7511_UP_CONVERSION_FIRST_ORDER = 1, +}; + +/** + * struct adv7511_link_config - Describes adv7511 hardware configuration + * @id: Video input format id + * @input_style: Video input format style + * @sync_pulse: Select the sync pulse + * @clock_delay: Clock delay for the input clock + * @reverse_bitorder: Reverse video input signal bitorder + * @bit_justification: Video input format bit justification + * @up_conversion: Selects the upscaling conversion method + * @input_color_depth: Input video format color depth + * @tmds_clock_inversion: Whether to invert the TDMS clock + * @vsync_polartity: vsync input signal configuration + * @hsync_polartity: hsync input signal configuration + * @timing_gen_seq: Selects the order in which sync DE generation + * and sync adjustment are performt. + * @gpio_pd: GPIO controlling the PD (powerdown) pin + **/ +struct adv7511_link_config { + enum adv7511_input_id id; + enum adv7511_input_style input_style; + enum adv7511_input_sync_pulse sync_pulse; + enum adv7511_input_clock_delay clock_delay; + bool reverse_bitorder; + enum adv7511_input_bit_justifiction bit_justification; + enum adv7511_up_conversion up_conversion; + enum adv7511_input_color_depth input_color_depth; + bool tmds_clock_inversion; + enum adv7511_timing_gen_seq timing_gen_seq; + + enum adv7511_sync_polarity vsync_polarity; + enum adv7511_sync_polarity hsync_polarity; + + int gpio_pd; + struct i2c_client **client; +}; + +/** + adi,input-style = 1|2|3; + adi,input-id = + "24-bit-rgb444-ycbcr444", + "16-20-24-bit-ycbcr422-separate-sync" | + "16-20-24-bit-ycbcr422-embedded-sync" | + "8-10-12-bit-ycbcr422-separate-sync" | + "8-10-12-bit-ycbcr422-embedded-sync" | + "12-15-16-bit-rgb444-ycbcr444" + adi,sync-pulse = "de","vsync","hsync","none" + adi,clock-delay = -1200|-800|-400|0|400|800|1200|1600 + adi,reverse-bitorder + adi,bit-justification = "left"|"right"|"evently"; + adi,up-conversion = "zero-order"|"first-order" + adi,input-color-depth = 8|10|12 + adi,tdms-clock-inversion + adi,vsync-polarity = "low"|"high"|"passthrough" + adi,hsync-polarity = "low"|"high"|"passtrhough" + adi,timing-gen-seq = "sync-adjustment-first"|"de-generation-first" +*/ + +/** + * enum adv7511_csc_scaling - Scaling factor for the ADV7511 CSC + * @ADV7511_CSC_SCALING_1: CSC results are not scaled + * @ADV7511_CSC_SCALING_2: CSC results are scaled by a factor of two + * @ADV7511_CSC_SCALING_4: CSC results are scalled by a factor of four + **/ +enum adv7511_csc_scaling { + ADV7511_CSC_SCALING_1 = 0, + ADV7511_CSC_SCALING_2 = 1, + ADV7511_CSC_SCALING_4 = 2, +}; + +/** + * struct adv7511_video_config - Describes adv7511 hardware configuration + * @csc_enable: Whether to enable color space conversion + * @csc_scaling_factor: Color space conversion scaling factor + * @csc_coefficents: Color space conversion coefficents + * @hdmi_mode: Whether to use HDMI or DVI output mode + * @avi_infoframe: HDMI infoframe + **/ +struct adv7511_video_config { + bool csc_enable; + enum adv7511_csc_scaling csc_scaling_factor; + const uint16_t *csc_coefficents; + + bool hdmi_mode; + struct hdmi_avi_infoframe avi_infoframe; +}; + +struct adv7511 { + struct i2c_client *i2c_main; + struct i2c_client *i2c_edid; + struct i2c_client *i2c_packet; + struct i2c_client *i2c_cec; + + struct regmap *regmap; + struct regmap *packet_memory_regmap; + enum drm_connector_status status; + int dpms_mode; + + unsigned int f_tmds; + unsigned int f_audio; + + unsigned int audio_source; + + unsigned int current_edid_segment; + uint8_t edid_buf[256]; + + wait_queue_head_t wq; + struct drm_encoder *encoder; + + bool embedded_sync; + enum adv7511_sync_polarity vsync_polarity; + enum adv7511_sync_polarity hsync_polarity; + + struct edid *edid; + + int gpio_pd; +}; + +struct edid *adv7511_get_edid(struct drm_encoder *encoder); + +#endif diff --git a/drivers/gpu/drm/i2c/adv7511_audio.c b/drivers/gpu/drm/i2c/adv7511_audio.c new file mode 100644 index 0000000..746f383 --- /dev/null +++ b/drivers/gpu/drm/i2c/adv7511_audio.c @@ -0,0 +1,304 @@ +/** + * Analog Devices ADV7511 HDMI transmitter driver + * + * Copyright 2012 Analog Devices Inc. + * + * Licensed under the GPL-2. + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/delay.h> +#include <linux/pm.h> +#include <linux/i2c.h> +#include <linux/spi/spi.h> +#include <linux/slab.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/initval.h> +#include <sound/tlv.h> + +#include "adv7511.h" + +static const struct snd_soc_dapm_widget adv7511_dapm_widgets[] = { + SND_SOC_DAPM_OUTPUT("TMDS"), + SND_SOC_DAPM_AIF_IN("AIFIN", "Playback", 0, SND_SOC_NOPM, 0, 0), +}; + +static const struct snd_soc_dapm_route adv7511_routes[] = { + { "TMDS", NULL, "AIFIN" }, +}; + +static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned int fs, + unsigned int *cts, unsigned int *n) +{ + switch (fs) { + case 32000: + *n = 4096; + break; + case 44100: + *n = 6272; + break; + case 48000: + *n = 6144; + break; + } + + *cts = ((f_tmds * *n) / (128 * fs)) * 1000; +} + +static int adv7511_update_cts_n(struct adv7511 *adv7511) +{ + unsigned int cts = 0; + unsigned int n = 0; + + adv7511_calc_cts_n(adv7511->f_tmds, adv7511->f_audio, &cts, &n); + + regmap_write(adv7511->regmap, ADV7511_REG_N0, (n >> 16) & 0xf); + regmap_write(adv7511->regmap, ADV7511_REG_N1, (n >> 8) & 0xff); + regmap_write(adv7511->regmap, ADV7511_REG_N2, n & 0xff); + + regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL0, (cts >> 16) & 0xf); + regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL1, (cts >> 8) & 0xff); + regmap_write(adv7511->regmap, ADV7511_REG_CTS_MANUAL2, cts & 0xff); + + return 0; +} + +static int adv7511_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_codec *codec = rtd->codec; + struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec); + unsigned int rate; + unsigned int len; + + switch (params_rate(params)) { + case 32000: + rate = ADV7511_SAMPLE_FREQ_32000; + break; + case 44100: + rate = ADV7511_SAMPLE_FREQ_44100; + break; + case 48000: + rate = ADV7511_SAMPLE_FREQ_48000; + break; + case 88200: + rate = ADV7511_SAMPLE_FREQ_88200; + break; + case 96000: + rate = ADV7511_SAMPLE_FREQ_96000; + break; + case 176400: + rate = ADV7511_SAMPLE_FREQ_176400; + break; + case 192000: + rate = ADV7511_SAMPLE_FREQ_192000; + break; + default: + return -EINVAL; + } + + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S16_LE: + len = ADV7511_I2S_SAMPLE_LEN_16; + break; + case SNDRV_PCM_FORMAT_S18_3LE: + len = ADV7511_I2S_SAMPLE_LEN_18; + break; + case SNDRV_PCM_FORMAT_S20_3LE: + len = ADV7511_I2S_SAMPLE_LEN_20; + break; + case SNDRV_PCM_FORMAT_S24_LE: + len = ADV7511_I2S_SAMPLE_LEN_24; + break; + default: + return -EINVAL; + } + + adv7511->f_audio = params_rate(params); + + adv7511_update_cts_n(adv7511); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CFG3, + ADV7511_AUDIO_CFG3_LEN_MASK, len); + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG, + ADV7511_I2C_FREQ_ID_CFG_RATE_MASK, rate << 4); + + return 0; +} + +static int adv7511_set_dai_fmt(struct snd_soc_dai *codec_dai, + unsigned int fmt) +{ + struct snd_soc_codec *codec = codec_dai->codec; + struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec); + unsigned int audio_source, i2s_format = 0; + unsigned int invert_clock; + + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_I2S: + audio_source = ADV7511_AUDIO_SOURCE_I2S; + i2s_format = ADV7511_I2S_FORMAT_I2S; + break; + case SND_SOC_DAIFMT_RIGHT_J: + audio_source = ADV7511_AUDIO_SOURCE_I2S; + i2s_format = ADV7511_I2S_FORMAT_RIGHT_J; + break; + case SND_SOC_DAIFMT_LEFT_J: + audio_source = ADV7511_AUDIO_SOURCE_I2S; + i2s_format = ADV7511_I2S_FORMAT_LEFT_J; + break; + case SND_SOC_DAIFMT_SPDIF: + audio_source = ADV7511_AUDIO_SOURCE_SPDIF; + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBS_CFS: + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + invert_clock = 0; + break; + case SND_SOC_DAIFMT_IB_NF: + invert_clock = 1; + break; + default: + return -EINVAL; + } + + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 0x70, audio_source << 4); + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(6), invert_clock << 6); + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 0x03, i2s_format); + + adv7511->audio_source = audio_source; + + return 0; +} + +static int adv7511_set_bias_level(struct snd_soc_codec *codec, + enum snd_soc_bias_level level) +{ + struct adv7511 *adv7511 = snd_soc_codec_get_drvdata(codec); + + switch (level) { + case SND_SOC_BIAS_ON: + switch (adv7511->audio_source) { + case ADV7511_AUDIO_SOURCE_I2S: + break; + case ADV7511_AUDIO_SOURCE_SPDIF: + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(7), BIT(7)); + break; + } + break; + case SND_SOC_BIAS_PREPARE: + if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) { + adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AUDIO_SAMPLE); + adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME); + adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_N_CTS); + } else { + adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AUDIO_SAMPLE); + adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AUDIO_INFOFRAME); + adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_N_CTS); + } + break; + case SND_SOC_BIAS_STANDBY: + regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, BIT(7), 0); + break; + case SND_SOC_BIAS_OFF: + break; + } + codec->dapm.bias_level = level; + return 0; +} + +#define ADV7511_RATES (SNDRV_PCM_RATE_32000 |\ + SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |\ + SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |\ + SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000) + +#define ADV7511_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S18_3LE |\ + SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S24_LE) + +static const struct snd_soc_dai_ops adv7511_dai_ops = { + .hw_params = adv7511_hw_params, + /*.set_sysclk = adv7511_set_dai_sysclk,*/ + .set_fmt = adv7511_set_dai_fmt, +}; + +static struct snd_soc_dai_driver adv7511_dai = { + .name = "adv7511", + .playback = { + .stream_name = "Playback", + .channels_min = 2, + .channels_max = 2, + .rates = ADV7511_RATES, + .formats = ADV7511_FORMATS, + }, + .ops = &adv7511_dai_ops, +}; + +static int adv7511_suspend(struct snd_soc_codec *codec) +{ + return adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF); +} + +static int adv7511_resume(struct snd_soc_codec *codec) +{ + return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY); +} + +static int adv7511_probe(struct snd_soc_codec *codec) +{ + int ret; + + ret = snd_soc_codec_set_cache_io(codec, 0, 0, SND_SOC_REGMAP); + if (ret < 0) { + dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret); + return ret; + } + + return adv7511_set_bias_level(codec, SND_SOC_BIAS_STANDBY); +} + +static int adv7511_remove(struct snd_soc_codec *codec) +{ + adv7511_set_bias_level(codec, SND_SOC_BIAS_OFF); + return 0; +} + +static struct snd_soc_codec_driver adv7511_codec_driver = { + .probe = adv7511_probe, + .remove = adv7511_remove, + .suspend = adv7511_suspend, + .resume = adv7511_resume, + .set_bias_level = adv7511_set_bias_level, + + .dapm_widgets = adv7511_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(adv7511_dapm_widgets), + .dapm_routes = adv7511_routes, + .num_dapm_routes = ARRAY_SIZE(adv7511_routes), +}; + +int adv7511_audio_init(struct device *dev) +{ + return snd_soc_register_codec(dev, &adv7511_codec_driver, + &adv7511_dai, 1); +} + +void adv7511_audio_exit(struct device *dev) +{ + snd_soc_unregister_codec(dev); +} diff --git a/drivers/gpu/drm/i2c/adv7511_core.c b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 index 0000000..8c7fe99 --- /dev/null +++ b/drivers/gpu/drm/i2c/adv7511_core.c @@ -0,0 +1,981 @@ +/** + * Analog Devices ADV7511 HDMI transmitter driver + * + * Copyright 2012 Analog Devices Inc. + * + * Licensed under the GPL-2. + */ + +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/regmap.h> +#include <linux/gpio.h> +#include <linux/of_gpio.h> + +#include "adv7511.h" + +#include <drm/drmP.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder_slave.h> +#include <drm/drm_edid.h> + +static const uint8_t adv7511_register_defaults[] = { + 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 00 */ + 0x00, 0x00, 0x01, 0x0e, 0xbc, 0x18, 0x01, 0x13, + 0x25, 0x37, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 10 */ + 0x46, 0x62, 0x04, 0xa8, 0x00, 0x00, 0x1c, 0x84, + 0x1c, 0xbf, 0x04, 0xa8, 0x1e, 0x70, 0x02, 0x1e, /* 20 */ + 0x00, 0x00, 0x04, 0xa8, 0x08, 0x12, 0x1b, 0xac, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 30 */ + 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0xb0, + 0x00, 0x50, 0x90, 0x7e, 0x79, 0x70, 0x00, 0x00, /* 40 */ + 0x00, 0xa8, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x02, 0x0d, 0x00, 0x00, 0x00, 0x00, /* 50 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 60 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x01, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 70 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* 80 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0xc0, 0x00, 0x00, 0x00, /* 90 */ + 0x0b, 0x02, 0x00, 0x18, 0x5a, 0x60, 0x00, 0x00, + 0x00, 0x00, 0x80, 0x80, 0x08, 0x04, 0x00, 0x00, /* a0 */ + 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x40, 0x14, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* b0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* c0 */ + 0x00, 0x03, 0x00, 0x00, 0x02, 0x00, 0x01, 0x04, + 0x30, 0xff, 0x80, 0x80, 0x80, 0x00, 0x00, 0x00, /* d0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x01, + 0x80, 0x75, 0x00, 0x00, 0x60, 0x00, 0x00, 0x00, /* e0 */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x75, 0x11, 0x00, /* f0 */ + 0x00, 0x7c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +}; + +/* ADI recommanded values for proper operation. */ +static const struct reg_default adv7511_fixed_registers[] = { + { 0x98, 0x03 }, + { 0x9a, 0xe0 }, + { 0x9c, 0x30 }, + { 0x9d, 0x61 }, + { 0xa2, 0xa4 }, + { 0xa3, 0xa4 }, + { 0xe0, 0xd0 }, + { 0xf9, 0x00 }, + { 0x55, 0x02 }, +}; + +static struct adv7511 *encoder_to_adv7511(struct drm_encoder *encoder) +{ + return to_encoder_slave(encoder)->slave_priv; +} + +static void adv7511_set_colormap(struct adv7511 *adv7511, bool enable, + const uint16_t *coeff, unsigned int scaling_factor) +{ + unsigned int i; + + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), + ADV7511_CSC_UPDATE_MODE, ADV7511_CSC_UPDATE_MODE); + + if (enable) { + for (i = 0; i < 12; ++i) { + regmap_update_bits(adv7511->regmap, + ADV7511_REG_CSC_UPPER(i), + 0x1f, coeff[i] >> 8); + regmap_write(adv7511->regmap, + ADV7511_REG_CSC_LOWER(i), + coeff[i] & 0xff); + } + } + + if (enable) { + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), + 0xe0, 0x80 | (scaling_factor << 5)); + } else { + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(0), + 0x80, 0x00); + } + + regmap_update_bits(adv7511->regmap, ADV7511_REG_CSC_UPPER(1), + ADV7511_CSC_UPDATE_MODE, 0); +} + +#define ADV7511_HDMI_CFG_MODE_MASK 0x2 +#define ADV7511_HDMI_CFG_MODE_DVI 0x0 +#define ADV7511_HDMI_CFG_MODE_HDMI 0x2 + +#define ADV7511_PACKET_MEM_SPD 0 +#define ADV7511_PACKET_MEM_MPEG 1 +#define ADV7511_PACKET_MEM_ACP 2 +#define ADV7511_PACKET_MEM_ISRC1 3 +#define ADV7511_PACKET_MEM_ISRC2 4 +#define ADV7511_PACKET_MEM_GM 5 +#define ADV7511_PACKET_MEM_SPARE1 6 +#define ADV7511_PACKET_MEM_SPARE2 7 + +#define ADV7511_PACKET_MEM_DATA_REG(x) ((x) * 0x20) +#define ADV7511_PACKET_MEM_UPDATE_REG(x) ((x) * 0x20 + 0x1f) +#define ADV7511_PACKET_MEM_UPDATE_ENABLE BIT(7) +#if 0 +static void adv7511_program_infoframe(struct adv7511 *adv7511, + const void *buffer, size_t size, unsigned int reg) +{ + unsigned int data_reg, update_reg; + unsigned int update_bit; + + switch (type) { + case AVI: + regmap = adv7511->regmap; + data_reg = ADV7511_REG_AVI_INFOFRAME_VERSION; + update_reg = ADV7511_REG_INFOFRAME_UPDATE; + update_bit = BIT(6); + break; + case AUDIO: + regmap = adv7511->regmap; + data_reg = ADV7511_REG_AUDIO_INFOFRAME_VERSION; + update_reg = ADV7511_REG_INFOFRAME_UPDATE; + update_bit = BIT(5); + break; + case GC: + regmap = adv7511->regmap; + data_reg = ADV7511_REG_GC(0); + update_reg = ADV7511_REG_INFOFRAME_UPDATE; + update_bit = BIT(4); + break; + case SPD: + regmap = adv7511->packet_memory_regmap; + data_reg = ADV7511_PACKET_MEM_DATA_REG(ADV7511_PACKET_MEM_SPD); + data_reg = ADV7511_PACKET_MEM_UPDATE_REG(ADV7511_PACKET_MEM_SPD); + update_bit = ADV7511_PACKET_MEM_UPDATE_ENABLE; + break; + } + + regmap_update_bits(adv7511->regmap, update_reg, update_bit, update_bit); + + regmap_bulk_write(adv7511->regmap, data_reg, buffer, size); + + regmap_update_bits(adv7511->regmap, update_reg, update_bit, 0); +} + +#endif + +static void adv7511_set_config(struct drm_encoder *encoder, void *c) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + struct adv7511_video_config *config = c; + bool output_format_422, output_format_ycbcr; + unsigned int mode; + uint8_t infoframe[17]; + + if (config->hdmi_mode) { + mode = ADV7511_HDMI_CFG_MODE_HDMI; + + switch (config->avi_infoframe.colorspace) { + case HDMI_COLORSPACE_YUV444: + output_format_422 = false; + output_format_ycbcr = true; + break; + case HDMI_COLORSPACE_YUV422: + output_format_422 = true; + output_format_ycbcr = true; + break; + default: + output_format_422 = false; + output_format_ycbcr = false; + break; + } + } else { + mode = ADV7511_HDMI_CFG_MODE_DVI; + output_format_422 = false; + output_format_ycbcr = false; + } + + adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); + + adv7511_set_colormap(adv7511, config->csc_enable, + config->csc_coefficents, config->csc_scaling_factor); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x81, + (output_format_422 << 7) | output_format_ycbcr); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, + ADV7511_HDMI_CFG_MODE_MASK, mode); + + hdmi_avi_infoframe_pack(&config->avi_infoframe, infoframe, + sizeof(infoframe)); + + /* The AVI infoframe id is not configurable */ + regmap_bulk_write(adv7511->regmap, ADV7511_REG_AVI_INFOFRAME_VERSION, + infoframe + 1, sizeof(infoframe) - 1); + + adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME); +} + +static void adv7511_set_link_config(struct adv7511 *adv7511, + const struct adv7511_link_config *config) +{ + enum adv7511_input_sync_pulse sync_pulse; + + switch (config->id) { + case ADV7511_INPUT_ID_12_15_16BIT_RGB444_YCbCr444: + sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE; + break; + default: + sync_pulse = config->sync_pulse; + break; + } + + switch (config->id) { + case ADV7511_INPUT_ID_16_20_24BIT_YCbCr422_EMBEDDED_SYNC: + case ADV7511_INPUT_ID_8_10_12BIT_YCbCr422_EMBEDDED_SYNC: + adv7511->embedded_sync = true; + break; + default: + adv7511->embedded_sync = false; + break; + } + + regmap_update_bits(adv7511->regmap, ADV7511_REG_I2C_FREQ_ID_CFG, + 0xf, config->id); + regmap_update_bits(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG1, 0x7e, + (config->input_color_depth << 4) | (config->input_style << 2)); + regmap_write(adv7511->regmap, ADV7511_REG_VIDEO_INPUT_CFG2, + (config->reverse_bitorder << 6) | + (config->bit_justification << 3)); + regmap_write(adv7511->regmap, ADV7511_REG_TIMING_GEN_SEQ, + (sync_pulse << 2) | + (config->timing_gen_seq << 1)); + regmap_write(adv7511->regmap, 0xba, + (config->clock_delay << 5)); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_TMDS_CLOCK_INV, + 0x08, config->tmds_clock_inversion << 3); + + adv7511->hsync_polarity = config->hsync_polarity; + adv7511->vsync_polarity = config->vsync_polarity; +} + +int adv7511_packet_enable(struct adv7511 *adv7511, unsigned int packet) +{ + if (packet & 0xff) { + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, + packet, 0xff); + } + + if (packet & 0xff00) { + packet >>= 8; + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, + packet, 0xff); + } + + return 0; +} + +int adv7511_packet_disable(struct adv7511 *adv7511, unsigned int packet) +{ + if (packet & 0xff) { + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE0, + packet, 0x00); + } + + if (packet & 0xff00) { + packet >>= 8; + regmap_update_bits(adv7511->regmap, ADV7511_REG_PACKET_ENABLE1, + packet, 0x00); + } + + return 0; +} + +static bool adv7511_register_volatile(struct device *dev, unsigned int reg) +{ + switch (reg) { + case ADV7511_REG_SPDIF_FREQ: + case ADV7511_REG_CTS_AUTOMATIC1: + case ADV7511_REG_CTS_AUTOMATIC2: + case ADV7511_REG_VIC_DETECTED: + case ADV7511_REG_VIC_SEND: + case ADV7511_REG_AUX_VIC_DETECTED: + case ADV7511_REG_STATUS: + case ADV7511_REG_GC(1): + case ADV7511_REG_INT(0): + case ADV7511_REG_INT(1): + case ADV7511_REG_PLL_STATUS: + case ADV7511_REG_AN(0): + case ADV7511_REG_AN(1): + case ADV7511_REG_AN(2): + case ADV7511_REG_AN(3): + case ADV7511_REG_AN(4): + case ADV7511_REG_AN(5): + case ADV7511_REG_AN(6): + case ADV7511_REG_AN(7): + case ADV7511_REG_HDCP_STATUS: + case ADV7511_REG_BCAPS: + case ADV7511_REG_BKSV(0): + case ADV7511_REG_BKSV(1): + case ADV7511_REG_BKSV(2): + case ADV7511_REG_BKSV(3): + case ADV7511_REG_BKSV(4): + case ADV7511_REG_DDC_STATUS: + case ADV7511_REG_BSTATUS(0): + case ADV7511_REG_BSTATUS(1): + case ADV7511_REG_CHIP_ID_HIGH: + case ADV7511_REG_CHIP_ID_LOW: + return true; + } + + return false; +} + +static bool adv7511_hpd(struct adv7511 *adv7511) +{ + unsigned int irq0; + int ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); + if (ret < 0) + return false; + + if (irq0 & ADV7511_INT0_HDP) { + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), ADV7511_INT0_HDP); + return true; + } + + return false; +} + +static irqreturn_t adv7511_irq_handler(int irq, void *devid) +{ + struct adv7511 *adv7511 = devid; + + if (adv7511_hpd(adv7511)) + drm_helper_hpd_irq_event(adv7511->encoder->dev); + + wake_up_all(&adv7511->wq); + + return IRQ_HANDLED; +} + +static unsigned int adv7511_is_interrupt_pending(struct adv7511 *adv7511, + unsigned int irq) +{ + unsigned int irq0, irq1; + unsigned int pending; + int ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0); + if (ret < 0) + return 0; + ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1); + if (ret < 0) + return 0; + + pending = (irq1 << 8) | irq0; + + return pending & irq; +} + +static int adv7511_wait_for_interrupt(struct adv7511 *adv7511, int irq, + int timeout) +{ + unsigned int pending = 0; + int ret; + + if (adv7511->i2c_main->irq) { + ret = wait_event_interruptible_timeout(adv7511->wq, + adv7511_is_interrupt_pending(adv7511, irq), + msecs_to_jiffies(timeout)); + if (ret <= 0) + return 0; + pending = adv7511_is_interrupt_pending(adv7511, irq); + } else { + if (timeout < 25) + timeout = 25; + do { + pending = adv7511_is_interrupt_pending(adv7511, irq); + if (pending) + break; + msleep(25); + timeout -= 25; + } while (timeout >= 25); + } + + return pending; +} + +static int adv7511_get_edid_block(void *data, unsigned char *buf, + int block, int len) +{ + struct drm_encoder *encoder = data; + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + struct i2c_msg xfer[2]; + uint8_t offset; + int i; + int ret; + + if (len > 128) + return -EINVAL; + + if (adv7511->current_edid_segment != block / 2) { + unsigned int status; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_DDC_STATUS, &status); + if (ret < 0) + return ret; + + if (status != 2) { + regmap_write(adv7511->regmap, ADV7511_REG_EDID_SEGMENT, block); + ret = adv7511_wait_for_interrupt(adv7511, ADV7511_INT0_EDID_READY | + ADV7511_INT1_DDC_ERROR, 200); + + if (!(ret & ADV7511_INT0_EDID_READY)) + return -EIO; + } + + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), + ADV7511_INT0_EDID_READY | ADV7511_INT1_DDC_ERROR); + + /* Break this apart, hopefully more I2C controllers will support 64 + * byte transfers than 256 byte transfers */ + + xfer[0].addr = adv7511->i2c_edid->addr; + xfer[0].flags = 0; + xfer[0].len = 1; + xfer[0].buf = &offset; + xfer[1].addr = adv7511->i2c_edid->addr; + xfer[1].flags = I2C_M_RD; + xfer[1].len = 64; + xfer[1].buf = adv7511->edid_buf; + + offset = 0; + + for (i = 0; i < 4; ++i) { + ret = i2c_transfer(adv7511->i2c_edid->adapter, xfer, ARRAY_SIZE(xfer)); + if (ret < 0) + return ret; + else if (ret != 2) + return -EIO; + + xfer[1].buf += 64; + offset += 64; + } + + adv7511->current_edid_segment = block / 2; + } + + if (block % 2 == 0) + memcpy(buf, adv7511->edid_buf, len); + else + memcpy(buf, adv7511->edid_buf + 128, len); + + return 0; +} + +static int adv7511_get_modes(struct drm_encoder *encoder, + struct drm_connector *connector) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + struct edid *edid; + unsigned int count; + + /* Reading the EDID only works if the device is powered */ + if (adv7511->dpms_mode != DRM_MODE_DPMS_ON) { + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), + ADV7511_INT0_EDID_READY | ADV7511_INT1_DDC_ERROR); + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, 0); + adv7511->current_edid_segment = -1; + } + + edid = drm_do_get_edid(connector, adv7511_get_edid_block, encoder); + + if (adv7511->dpms_mode != DRM_MODE_DPMS_ON) + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); + + adv7511->edid = edid; + if (!edid) + return 0; + + drm_mode_connector_update_edid_property(connector, edid); + count = drm_add_edid_modes(connector, edid); + + kfree(adv7511->edid); + + return count; +} + +struct edid *adv7511_get_edid(struct drm_encoder *encoder) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + + if (!adv7511->edid) + return NULL; + + return kmemdup(adv7511->edid, sizeof(*adv7511->edid) + + adv7511->edid->extensions * 128, GFP_KERNEL); +} +EXPORT_SYMBOL_GPL(adv7511_get_edid); + +static void adv7511_encoder_dpms(struct drm_encoder *encoder, int mode) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + + switch (mode) { + case DRM_MODE_DPMS_ON: + adv7511->current_edid_segment = -1; + + regmap_write(adv7511->regmap, ADV7511_REG_INT(0), + ADV7511_INT0_EDID_READY | ADV7511_INT1_DDC_ERROR); + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, 0); + /* + * Per spec it is allowed to pulse the HDP signal to indicate + * that the EDID information has changed. Some monitors do this + * when they wakeup from standby or are enabled. When the HDP + * goes low the adv7511 is reset and the outputs are disabled + * which might cause the monitor to go to standby again. To + * avoid this we ignore the HDP pin for the first few seconds + * after enabeling the output. + */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, + ADV7511_REG_POWER2_HDP_SRC_MASK, + ADV7511_REG_POWER2_HDP_SRC_NONE); + /* Most of the registers are reset during power down or when HPD is low */ + regcache_sync(adv7511->regmap); + break; + default: + /* TODO: setup additional power down modes */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); + regcache_mark_dirty(adv7511->regmap); + break; + } + + adv7511->dpms_mode = mode; +} + +static enum drm_connector_status adv7511_encoder_detect(struct drm_encoder *encoder, + struct drm_connector *connector) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + enum drm_connector_status status; + unsigned int val; + bool hpd; + int ret; + + ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, &val); + if (ret < 0) + return connector_status_disconnected; + + if (val & ADV7511_STATUS_HPD) + status = connector_status_connected; + else + status = connector_status_disconnected; + + hpd = adv7511_hpd(adv7511); + + /* The chip resets itself when the cable is disconnected, so in case + * there is a pending HPD interrupt and the cable is connected there was + * at least on transition from disconnected to connected and the chip + * has to be reinitialized. */ + if (status == connector_status_connected && hpd && + adv7511->dpms_mode == DRM_MODE_DPMS_ON) { + regcache_mark_dirty(adv7511->regmap); + adv7511_encoder_dpms(encoder, adv7511->dpms_mode); + adv7511_get_modes(encoder, connector); + status = connector_status_disconnected; + } else { + /* Renable HDP sensing */ + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, + ADV7511_REG_POWER2_HDP_SRC_MASK, + ADV7511_REG_POWER2_HDP_SRC_BOTH); + } + + adv7511->status = status; + return status; +} + +static void adv7511_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adj_mode) +{ + struct adv7511 *adv7511 = encoder_to_adv7511(encoder); + unsigned int low_refresh_rate; + unsigned int hsync_polarity = 0; + unsigned int vsync_polarity = 0; + + if (adv7511->embedded_sync) { + unsigned int hsync_offset, hsync_len; + unsigned int vsync_offset, vsync_len; + + hsync_offset = adj_mode->crtc_hsync_start - adj_mode->crtc_hdisplay; + vsync_offset = adj_mode->crtc_vsync_start - adj_mode->crtc_vdisplay; + hsync_len = adj_mode->crtc_hsync_end - adj_mode->crtc_hsync_start; + vsync_len = adj_mode->crtc_vsync_end - adj_mode->crtc_vsync_start; + + /* The hardware vsync generator has a off-by-one bug */ + vsync_offset += 1; + + regmap_write(adv7511->regmap, ADV7511_REG_HSYNC_PLACEMENT_MSB, + ((hsync_offset >> 10) & 0x7) << 5); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(0), + (hsync_offset >> 2) & 0xff); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(1), + ((hsync_offset & 0x3) << 6) | ((hsync_len >> 4) & 0x3f)); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(2), + ((hsync_len & 0xf) << 4) | ((vsync_offset >> 6) & 0xf)); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(3), + ((vsync_offset & 0x3f) << 2) | ((vsync_len >> 8) & 0x3)); + regmap_write(adv7511->regmap, ADV7511_REG_SYNC_DECODER(4), + vsync_len & 0xff); + + hsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PHSYNC); + vsync_polarity = !(adj_mode->flags & DRM_MODE_FLAG_PVSYNC); + } else { + enum adv7511_sync_polarity mode_hsync_polarity; + enum adv7511_sync_polarity mode_vsync_polarity; + + /** + * If the input signal is always low or always high we want to + * invert or let it passthrough depending on the polarity of the + * current mode. + **/ + if (adj_mode->flags & DRM_MODE_FLAG_NHSYNC) + mode_hsync_polarity = ADV7511_SYNC_POLARITY_LOW; + else + mode_hsync_polarity = ADV7511_SYNC_POLARITY_HIGH; + + if (adj_mode->flags & DRM_MODE_FLAG_NVSYNC) + mode_vsync_polarity = ADV7511_SYNC_POLARITY_LOW; + else + mode_vsync_polarity = ADV7511_SYNC_POLARITY_HIGH; + + if (adv7511->hsync_polarity != mode_hsync_polarity && + adv7511->hsync_polarity != ADV7511_SYNC_POLARITY_PASSTHROUGH) + hsync_polarity = 1; + + if (adv7511->vsync_polarity != mode_vsync_polarity && + adv7511->vsync_polarity != ADV7511_SYNC_POLARITY_PASSTHROUGH) + vsync_polarity = 1; + } + + if (mode->vrefresh <= 24000) + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_24HZ; + else if (mode->vrefresh <= 25000) + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_25HZ; + else if (mode->vrefresh <= 30000) + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_30HZ; + else + low_refresh_rate = ADV7511_LOW_REFRESH_RATE_NONE; + + regmap_update_bits(adv7511->regmap, 0xfb, + 0x6, low_refresh_rate << 1); + regmap_update_bits(adv7511->regmap, 0x17, + 0x60, (vsync_polarity << 6) | (hsync_polarity << 5)); + + adv7511->f_tmds = mode->clock; +} + +static struct drm_encoder_slave_funcs adv7511_encoder_funcs = { + .set_config = adv7511_set_config, + .dpms = adv7511_encoder_dpms, + /* .destroy = adv7511_encoder_destroy,*/ + .mode_set = adv7511_encoder_mode_set, + .detect = adv7511_encoder_detect, + .get_modes = adv7511_get_modes, +}; + +static const struct regmap_config adv7511_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + + .max_register = 0xff, + .cache_type = REGCACHE_RBTREE, + .reg_defaults_raw = adv7511_register_defaults, + .num_reg_defaults_raw = ARRAY_SIZE(adv7511_register_defaults), + + .volatile_reg = adv7511_register_volatile, +}; + +/* + adi,input-id - + 0x00: + 0x01: + 0x02: + 0x03: + 0x04: + 0x05: + adi,sync-pulse - Selects the sync pulse + 0x00: Use the DE signal as sync pulse + 0x01: Use the HSYNC signal as sync pulse + 0x02: Use the VSYNC signal as sync pulse + 0x03: No external sync pulse + adi,bit-justification - + 0x00: Evently + 0x01: Right + 0x02: Left + adi,up-conversion - + 0x00: zero-order up conversion + 0x01: first-order up conversion + adi,timing-generation-sequence - + 0x00: Sync adjustment first, then DE generation + 0x01: DE generation first then sync adjustment + adi,vsync-polarity - Polarity of the vsync signal + 0x00: Passthrough + 0x01: Active low + 0x02: Active high + adi,hsync-polarity - Polarity of the hsync signal + 0x00: Passthrough + 0x01: Active low + 0x02: Active high + adi,reverse-bitorder - If set the bitorder is reveresed + adi,tmds-clock-inversion - If set use tdms clock inversion + adi,clock-delay - Clock delay for the video data clock + 0x00: -1200 ps + 0x01: -800 ps + 0x02: -400 ps + 0x03: no dealy + 0x04: 400 ps + 0x05: 800 ps + 0x06: 1200 ps + 0x07: 1600 ps + adi,input-style - Specifies the input style used + 0x02: Use input style 1 + 0x01: Use input style 2 + 0x03: Use Input style 3 + adi,input-color-depth - Selects the input format color depth + 0x03: 8-bit per channel + 0x01: 10-bit per channel + 0x02: 12-bit per channel +*/ + + +static int adv7511_parse_dt(struct device_node *np, struct adv7511_link_config *config) +{ + int ret; + u32 val; + + ret = of_property_read_u32(np, "adi,input-id", &val); + if (ret < 0) + return ret; + config->id = val; + + ret = of_property_read_u32(np, "adi,sync-pulse", &val); + if (ret < 0) + config->sync_pulse = ADV7511_INPUT_SYNC_PULSE_NONE; + else + config->sync_pulse = val; + + ret = of_property_read_u32(np, "adi,bit-justification", &val); + if (ret < 0) + return ret; + config->bit_justification = val; + + ret = of_property_read_u32(np, "adi,up-conversion", &val); + if (ret < 0) + config->up_conversion = ADV7511_UP_CONVERSION_ZERO_ORDER; + else + config->up_conversion = val; + + ret = of_property_read_u32(np, "adi,timing-generation-sequence", &val); + if (ret < 0) + return ret; + config->timing_gen_seq = val; + + ret = of_property_read_u32(np, "adi,vsync-polarity", &val); + if (ret < 0) + return ret; + config->vsync_polarity = val; + + ret = of_property_read_u32(np, "adi,hsync-polarity", &val); + if (ret < 0) + return ret; + config->hsync_polarity = val; + + config->reverse_bitorder = of_property_read_bool(np, + "adi,reverse-bitorder"); + config->tmds_clock_inversion = of_property_read_bool(np, + "adi,tmds-clock-inversion"); + + ret = of_property_read_u32(np, "adi,clock-delay", &val); + if (ret) + return ret; + config->clock_delay = val; + + ret = of_property_read_u32(np, "adi,input-style", &val); + if (ret) + return ret; + config->input_style = val; + + ret = of_property_read_u32(np, "adi,input-color-depth", &val); + if (ret) + return ret; + config->input_color_depth = val; + + config->gpio_pd = of_get_gpio(np, 0); + if (config->gpio_pd == -EPROBE_DEFER) + return -EPROBE_DEFER; + + return 0; +} + +static const int edid_i2c_addr = 0x7e; +static const int packet_i2c_addr = 0x70; +static const int cec_i2c_addr = 0x78; + +static int adv7511_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + struct adv7511_link_config link_config; + struct adv7511 *adv7511; + unsigned int val; + int ret; + + if (i2c->dev.of_node) { + ret = adv7511_parse_dt(i2c->dev.of_node, &link_config); + if (ret) + return ret; + } else { + if (!i2c->dev.platform_data) + return -EINVAL; + link_config = *(struct adv7511_link_config *)i2c->dev.platform_data; + } + + adv7511 = devm_kzalloc(&i2c->dev, sizeof(*adv7511), GFP_KERNEL); + if (!adv7511) + return -ENOMEM; + + adv7511->gpio_pd = link_config.gpio_pd; + + if (gpio_is_valid(adv7511->gpio_pd)) { + ret = devm_gpio_request_one(&i2c->dev, adv7511->gpio_pd, + GPIOF_OUT_INIT_HIGH, "PD"); + if (ret) + return ret; + mdelay(5); + gpio_set_value_cansleep(adv7511->gpio_pd, 0); + } + + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); + if (IS_ERR(adv7511->regmap)) + return PTR_ERR(adv7511->regmap); + + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); + if (ret) + return ret; + dev_dbg(&i2c->dev, "Rev. %d\n", val); + + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, + ARRAY_SIZE(adv7511_fixed_registers)); + if (ret) + return ret; + + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, packet_i2c_addr); + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); + adv7511_packet_disable(adv7511, 0xffff); + + adv7511->i2c_main = i2c; + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); + adv7511->i2c_packet = i2c_new_dummy(i2c->adapter, packet_i2c_addr >> 1); + if (!adv7511->i2c_edid) + return -ENOMEM; + + if (i2c->irq) { + ret = request_threaded_irq(i2c->irq, NULL, adv7511_irq_handler, + IRQF_ONESHOT, dev_name(&i2c->dev), adv7511); + if (ret) + goto err_i2c_unregister_device; + + init_waitqueue_head(&adv7511->wq); + } + + /* CEC is unused for now */ + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, + ADV7511_CEC_CTRL_POWER_DOWN); + + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, + ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); + + adv7511->current_edid_segment = -1; + + i2c_set_clientdata(i2c, adv7511); + adv7511_audio_init(&i2c->dev); + + adv7511_set_link_config(adv7511, &link_config); + if (link_config.client) + *link_config.client = i2c; + + return 0; + +err_i2c_unregister_device: + i2c_unregister_device(adv7511->i2c_edid); + + return ret; +} + +static int adv7511_remove(struct i2c_client *i2c) +{ + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); + + i2c_unregister_device(adv7511->i2c_edid); + + if (i2c->irq) + free_irq(i2c->irq, adv7511); + kfree(adv7511->edid); + + return 0; +} + +static int adv7511_encoder_init(struct i2c_client *i2c, + struct drm_device *dev, struct drm_encoder_slave *encoder) +{ + + struct adv7511 *adv7511 = i2c_get_clientdata(i2c); + + encoder->slave_priv = adv7511; + encoder->slave_funcs = &adv7511_encoder_funcs; + + adv7511->encoder = &encoder->base; + + return 0; +} + +static const struct i2c_device_id adv7511_ids[] = { + { "adv7511", 0 }, + {} +}; + +static struct drm_i2c_encoder_driver adv7511_driver = { + .i2c_driver = { + .driver = { + .name = "adv7511", + }, + .id_table = adv7511_ids, + .probe = adv7511_probe, + .remove = adv7511_remove, + }, + + .encoder_init = adv7511_encoder_init, +}; + +static int __init adv7511_init(void) +{ + return drm_i2c_encoder_register(THIS_MODULE, &adv7511_driver); +} +module_init(adv7511_init); + +static void __exit adv7511_exit(void) +{ + drm_i2c_encoder_unregister(&adv7511_driver); +} +module_exit(adv7511_exit); + +MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); +MODULE_DESCRIPTION("ADV7511 HDMI transmitter driver"); +MODULE_LICENSE("GPL");
ADV7511 driver snapshot taken from commit f416e32 of xcomm_zynq_3_10 branch at https://github.com/analogdevicesinc/linux.git Changed to export its i2c_client handle via platform_data, which is the only way I could come up with to use a DRM encoder that is not attached to a dedicated on-GPU i2c bus. Signed-off-by: Ulrich Hecht <ulrich.hecht@gmail.com> --- drivers/gpu/drm/Kconfig | 6 + drivers/gpu/drm/i2c/Makefile | 3 + drivers/gpu/drm/i2c/adv7511.h | 455 +++++++++++++++++ drivers/gpu/drm/i2c/adv7511_audio.c | 304 +++++++++++ drivers/gpu/drm/i2c/adv7511_core.c | 981 ++++++++++++++++++++++++++++++++++++ 5 files changed, 1749 insertions(+) create mode 100644 drivers/gpu/drm/i2c/adv7511.h create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c