Hi Shashank, On 07-02-2017 16:19, Sharma, Shashank wrote: > Regards > > Shashank > > > On 2/7/2017 4:44 PM, Jose Abreu wrote: >> Hi Shashank, >> >> >> >> On 06-02-2017 13:59, Shashank Sharma wrote: >>> HDMI 2.0 spec mandates scrambling for modes with pixel clock >>> higher >>> than 340 MHz. This patch adds few new functions in drm layer for >>> core drivers to enable/disable scrambling. >>> >>> This patch adds: >>> - A function to detect scrambling support parsing HF-VSDB >>> - A function to check scrambling status runtime using SCDC read. >>> - Two functions to enable/disable scrambling using SCDC >>> read/write. >>> - Few new bools to reflect scrambling support and status. >>> >>> V2: Addressed review comments from Thierry, Ville and Dhinakaran >>> Thierry: >>> - Mhz -> MHz in comments and commit message. >>> - i2c -> I2C in comments. >>> - Fix the function documentations, keep in sync with >>> drm_scdc_helper.c >>> - drm_connector -> DRM connector. >>> - Create structure for SCDC, and save scrambling status >>> inside that, >>> in a sub-structure. >>> - Call this sub-structure scrambling instead of scr_info. >>> - low_rates -> low_clocks in scrambling status structure. >>> - Store the return value of I2C read/write and print the >>> error code >>> in case of failure. >>> >>> Thierry and Ville: >>> - Move the scrambling enable/disable/query functions in >>> drm_scdc_helper.c file. >>> - Add drm_SCDC prefix for the functions. >>> - Optimize the return statement from function >>> drm_SCDC_check_scrambling_status. >>> >>> Ville: >>> - Dont overwrite saved max TMDS clock value in display_info, >>> if max tmds clock from HF-VSDB is not > 340 MHz. >>> - drm_detect_hdmi_scrambling -> drm_parse_hdmi_forum_vsdb. >>> - Remove dynamic tracking of SCDC status from DRM layer, >>> force bool. >>> - Program clock ratio bit also, while enabling scrambling. >>> >>> Dhinakaran: >>> - Add a comment about *5000 while extracting max clock >>> supported. >>> >>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/drm_edid.c | 33 ++++++++++++- >>> drivers/gpu/drm/drm_scdc_helper.c | 100 >>> ++++++++++++++++++++++++++++++++++++++ >>> include/drm/drm_connector.h | 19 ++++++++ >>> include/drm/drm_edid.h | 6 ++- >>> include/drm/drm_scdc_helper.h | 20 ++++++++ >>> 5 files changed, 176 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_edid.c >>> b/drivers/gpu/drm/drm_edid.c >>> index a487b80..dc85eb9 100644 >>> --- a/drivers/gpu/drm/drm_edid.c >>> +++ b/drivers/gpu/drm/drm_edid.c >>> @@ -37,6 +37,7 @@ >>> #include <drm/drm_edid.h> >>> #include <drm/drm_encoder.h> >>> #include <drm/drm_displayid.h> >>> +#include <drm/drm_scdc_helper.h> >>> #define version_greater(edid, maj, min) \ >>> (((edid)->version > (maj)) || \ >>> @@ -3805,13 +3806,43 @@ enum hdmi_quantization_range >>> static void drm_parse_hdmi_forum_vsdb(struct drm_connector >>> *connector, >>> const u8 *hf_vsdb) >>> { >>> - struct drm_hdmi_info *hdmi = &connector->display_info.hdmi; >>> + struct drm_display_info *display = >>> &connector->display_info; >>> + struct drm_hdmi_info *hdmi = &display->hdmi; >>> if (hf_vsdb[6] & 0x80) { >>> hdmi->scdc.supported = true; >>> if (hf_vsdb[6] & 0x40) >>> hdmi->scdc.read_request = true; >>> } >>> + >>> + /* >>> + * All HDMI 2.0 monitors must support scrambling at >>> rates > 340 MHz. >>> + * And as per the spec, three factors confirm this: >>> + * * Availability of a HF-VSDB block in EDID (check) >>> + * * Non zero Max_TMDS_Char_Rate filed in HF-VSDB (let's >>> check) >>> + * * SCDC support available (let's check) >>> + * Lets check it out. >>> + */ >>> + >>> + if (hf_vsdb[5]) { >>> + /* max clock is 5000 KHz times block value */ >>> + u32 max_tmds_clock = hf_vsdb[5] * 5000; >>> + struct drm_scdc *scdc = &hdmi->scdc; >>> + >>> + if (max_tmds_clock > 340000) { >>> + display->max_tmds_clock = max_tmds_clock; >>> + DRM_DEBUG_KMS("HF-VSDB: max TMDS clock %d kHz\n", >>> + display->max_tmds_clock); >>> + } >>> + >>> + if (scdc->supported) { >>> + scdc->scrambling.supported = true; >>> + >>> + /* Few sinks support scrambling for cloks < 340M */ >>> + if ((hf_vsdb[6] & 0x8)) >> BIT(3) ? > Yes, bit 3 is LTE_340Mcsc_scramble, indicating that the sink > support scrambling at rates below 340Mhz too, isn't it ? >> >>> + scdc->scrambling.low_rates = true; >>> + } >>> + } >>> } >>> static void drm_parse_hdmi_deep_color_info(struct >>> drm_connector *connector, >>> diff --git a/drivers/gpu/drm/drm_scdc_helper.c >>> b/drivers/gpu/drm/drm_scdc_helper.c >>> index fe0e121..311f62e 100644 >>> --- a/drivers/gpu/drm/drm_scdc_helper.c >>> +++ b/drivers/gpu/drm/drm_scdc_helper.c >>> @@ -22,8 +22,10 @@ >>> */ >>> #include <linux/slab.h> >>> +#include <linux/delay.h> >>> #include <drm/drm_scdc_helper.h> >>> +#include <drm/drmP.h> >>> /** >>> * DOC: scdc helpers >>> @@ -109,3 +111,101 @@ ssize_t drm_scdc_write(struct >>> i2c_adapter *adapter, u8 offset, >>> return err; >>> } >>> EXPORT_SYMBOL(drm_scdc_write); >>> + >>> +/** >>> + * drm_scdc_check_scrambling_status - what is status of >>> scrambling? >>> + * @adapter: I2C adapter for DDC channel >>> + * >>> + * Reads the scrambler status over SCDC, and checks the >>> + * scrambling status. >>> + * >>> + * Returns: >>> + * True if the scrambling is enabled, false otherwise. >>> + */ >>> + >>> +bool drm_scdc_check_scrambling_status(struct i2c_adapter >>> *adapter) >>> +{ >>> + u8 status; >>> + int ret; >>> + >>> + ret = drm_scdc_readb(adapter, SCDC_SCRAMBLER_STATUS, >>> &status); >>> + if (ret < 0) { >>> + DRM_ERROR("Failed to read scrambling status, error >>> %d\n", ret); >>> + return false; >>> + } >>> + >>> + return status & SCDC_SCRAMBLING_STATUS; >> "return (status & SCDC_SCRAMBLING_STATUS) > 0;" ? > I think Jani has made an agreement already on this. >> >>> +} >>> +EXPORT_SYMBOL(drm_scdc_check_scrambling_status); >>> + >>> +/** >>> + * drm_scdc_enable_scrambling - enable scrambling >>> + * @adapter: I2C adapter for DDC channel >>> + * >>> + * Writes the TMDS config over SCDC channel, and enables >>> scrambling >>> + * Returns: >>> + * True if scrambling is successfully enabled, false otherwise. >>> + */ >>> + >>> +bool drm_scdc_enable_scrambling(struct i2c_adapter *adapter) >>> +{ >>> + u8 config; >>> + int ret; >>> + >>> + ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config); >>> + if (ret < 0) { >>> + DRM_ERROR("Failed to read tmds config, err=%d\n", ret); >>> + return false; >>> + } >>> + >>> + config |= (SCDC_SCRAMBLING_ENABLE | >>> SCDC_TMDS_BIT_CLOCK_RATIO_BY_40); >> Hmm, I did not read the spec exhaustively but shouldn't the clock >> ratio by 40 only be set for data rates above 3.4Gbps? > You are right, for few monitors scrambling can be done below > 340 MHz too, and I am not sure if we should > set the clock ratio bit on that. Let me check the spec for > those cases. >>> + >>> + ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config); >>> + if (ret < 0) { >>> + DRM_ERROR("Failed to enable scrambling, error %d\n", >>> ret); >>> + return false; >>> + } >>> + >>> + /* >>> + * The spec says that the source should wait min 1ms and >>> max 100ms >>> + * after writing the TMDS config for clock ratio. Lets >>> obey the spec. >>> + */ >>> + usleep_range(1000, 100000); >> Shall we read here the clock_detected status to make sure the >> sink is okay? > This is optional in spec, so I am afraid few monitors wont > implement this, and we will unnecessary add > lot of noise in code. Do you think so ? Hmm, ok. It was the safest thing to do but if we have monitors with scrambling support and without this then its better not to. Best regards, Jose Miguel Abreu > > - Shashank >>> + return true; >>> +} >>> +EXPORT_SYMBOL(drm_scdc_enable_scrambling); >>> + >>> +/** >>> + * drm_scdc_disable_scrambling - disable scrambling >>> + * @adapter: I2C adapter for DDC channel >>> + * >>> + * Write the TMDS config over SCDC channel, and disable >>> scrambling >>> + * Return: True if scrambling is successfully disabled, >>> false otherwise. >>> + */ >>> +bool drm_scdc_disable_scrambling(struct i2c_adapter *adapter) >>> +{ >>> + u8 config; >>> + int ret; >>> + >>> + ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config); >>> + if (ret < 0) { >>> + DRM_ERROR("Failed to read tmds config, error %d\n", >>> ret); >>> + return false; >>> + } >>> + >>> + config &= ~(SCDC_SCRAMBLING_ENABLE | >>> SCDC_TMDS_BIT_CLOCK_RATIO_BY_40); >>> + >>> + ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config); >>> + if (ret < 0) { >>> + DRM_ERROR("Failed to enable scrambling, error %d\n", >>> ret); >>> + return false; >>> + } >>> + >>> + /* >>> + * The spec says that the source should wait min 1ms and >>> max 100ms >>> + * after writing the TMDS config for clock ratio. Lets >>> obey the spec. >>> + */ >>> + usleep_range(1000, 100000); >>> + return true; >>> +} >>> +EXPORT_SYMBOL(drm_scdc_disable_scrambling); >>> diff --git a/include/drm/drm_connector.h >>> b/include/drm/drm_connector.h >>> index 6d5304e..78618308 100644 >>> --- a/include/drm/drm_connector.h >>> +++ b/include/drm/drm_connector.h >>> @@ -90,6 +90,20 @@ enum subpixel_order { >>> }; >>> +/** >>> + * struct drm_scrambling: sink's scrambling support. >>> + */ >>> +struct drm_scrambling { >>> + /** >>> + * @supported: scrambling supported for rates > 340 Mhz. >>> + */ >>> + bool supported; >>> + /** >>> + * @low_rates: scrambling supported for rates <= 340 Mhz. >>> + */ >>> + bool low_rates; >>> +}; >>> + >>> /* >>> * struct drm_scdc - Information about scdc capabilities of >>> a HDMI 2.0 sink >>> * >>> @@ -105,8 +119,13 @@ struct drm_scdc { >>> * @read_request: sink is capable of generating scdc >>> read request. >>> */ >>> bool read_request; >>> + /** >>> + * @scrambling: sink's scrambling capabilities >>> + */ >>> + struct drm_scrambling scrambling; >>> }; >>> + >>> /** >>> * struct drm_hdmi_info - runtime information about the >>> connected HDMI sink >>> * >>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h >>> index 43fb0ac..d24c974 100644 >>> --- a/include/drm/drm_edid.h >>> +++ b/include/drm/drm_edid.h >>> @@ -462,5 +462,9 @@ void drm_edid_get_monitor_name(struct >>> edid *edid, char *name, >>> struct drm_display_mode *drm_mode_find_dmt(struct >>> drm_device *dev, >>> int hsize, int vsize, int fresh, >>> bool rb); >>> - >>> +bool drm_enable_scrambling(struct drm_connector *connector, >>> + struct i2c_adapter *adapter, bool force); >>> +bool drm_disable_scrambling(struct drm_connector *connector, >>> + struct i2c_adapter *adapter, bool force); >>> +bool drm_check_scrambling_status(struct i2c_adapter *adapter); >>> #endif /* __DRM_EDID_H__ */ >>> diff --git a/include/drm/drm_scdc_helper.h >>> b/include/drm/drm_scdc_helper.h >>> index 93b07bc..dc727a5 100644 >>> --- a/include/drm/drm_scdc_helper.h >>> +++ b/include/drm/drm_scdc_helper.h >>> @@ -129,4 +129,24 @@ static inline int drm_scdc_writeb(struct >>> i2c_adapter *adapter, u8 offset, >>> return drm_scdc_write(adapter, offset, &value, >>> sizeof(value)); >>> } >>> +/** >>> + * drm_scdc_enable_scrambling - enable scrambling >>> + * @adapter: I2C adapter for DDC channel >>> + * >>> + * Writes the TMDS config over SCDC channel, and enables >>> scrambling >>> + * Returns: >>> + * True if scrambling is successfully enabled, false otherwise. >>> + */ >>> + >>> +bool drm_scdc_enable_scrambling(struct i2c_adapter *adapter); >>> + >>> +/** >>> + * drm_scdc_disable_scrambling - disable scrambling >>> + * @adapter: I2C adapter for DDC channel >>> + * >>> + * Write the TMDS config over SCDC channel, and disable >>> scrambling >>> + * Return: True if scrambling is successfully disabled, >>> false otherwise. >>> + */ >>> +bool drm_scdc_disable_scrambling(struct i2c_adapter *adapter); >>> + >>> #endif >> Best regards, >> Jose Miguel Abreu >> > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx