| Message ID | 1486389566-28613-5-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > --- > 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) ? > + 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;" ? > +} > +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? > + > + 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? > + 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
On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >> +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;" ? What's the point in that? BR, Jani.
Hi Jani, On 07-02-2017 13:35, Jani Nikula wrote: > On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >>> +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;" ? > What's the point in that? > > BR, > Jani. > > Sorry, I didn't see the SCDC_SCRAMBLING_STATUS is BIT(0). Anyway, my intention was to return either 1 or 0 or else the value of the "and" would be returned. I think in x86 the bool is char, what could happen if SCDC_SCRAMBLING_STATUS was BIT(>7)? Does the cast work as expected? Best regards, Jose Miguel Abreu
On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: > Hi Jani, > > > On 07-02-2017 13:35, Jani Nikula wrote: >> On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >>>> +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;" ? >> What's the point in that? >> >> BR, >> Jani. >> >> > > Sorry, I didn't see the SCDC_SCRAMBLING_STATUS is BIT(0). Anyway, > my intention was to return either 1 or 0 or else the value of the > "and" would be returned. I think in x86 the bool is char, what > could happen if SCDC_SCRAMBLING_STATUS was BIT(>7)? Does the cast > work as expected? The implicit type conversion works just fine. BR, Jani. > > Best regards, > Jose Miguel Abreu >
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@intel.com> >> --- >> 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 ? - 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 >
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@intel.com> >>> --- >>> 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 >> > >
Hi Jani, On 07-02-2017 15:09, Jani Nikula wrote: > On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >> Hi Jani, >> >> >> On 07-02-2017 13:35, Jani Nikula wrote: >>> On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >>>>> +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;" ? >>> What's the point in that? >>> >>> BR, >>> Jani. >>> >>> >> Sorry, I didn't see the SCDC_SCRAMBLING_STATUS is BIT(0). Anyway, >> my intention was to return either 1 or 0 or else the value of the >> "and" would be returned. I think in x86 the bool is char, what >> could happen if SCDC_SCRAMBLING_STATUS was BIT(>7)? Does the cast >> work as expected? > The implicit type conversion works just fine. Hmm, are you sure? I'm reading this thread: http://yarchive.net/comp/linux/bool.html (see Linus last answer). (This is just for curiosity anyway). Best regards, Jose Miguel Abreu > > BR, > Jani. > >> Best regards, >> Jose Miguel Abreu >>
On Wed, 08 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: > Hi Jani, > > > > On 07-02-2017 15:09, Jani Nikula wrote: >> On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >>> Hi Jani, >>> >>> >>> On 07-02-2017 13:35, Jani Nikula wrote: >>>> On Tue, 07 Feb 2017, Jose Abreu <Jose.Abreu@synopsys.com> wrote: >>>>>> +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;" ? >>>> What's the point in that? >>>> >>>> BR, >>>> Jani. >>>> >>>> >>> Sorry, I didn't see the SCDC_SCRAMBLING_STATUS is BIT(0). Anyway, >>> my intention was to return either 1 or 0 or else the value of the >>> "and" would be returned. I think in x86 the bool is char, what >>> could happen if SCDC_SCRAMBLING_STATUS was BIT(>7)? Does the cast >>> work as expected? >> The implicit type conversion works just fine. > > Hmm, are you sure? I'm reading this thread: > http://yarchive.net/comp/linux/bool.html (see Linus last answer). I think you're confusing ABI with what C guarantees. I don't think that thread has any relevance here. BR, Jani. > > (This is just for curiosity anyway). > > Best regards, > Jose Miguel Abreu > >> >> BR, >> Jani. >> >>> Best regards, >>> Jose Miguel Abreu >>> >
Regards Shashank On 2/8/2017 5:01 PM, Jose Abreu wrote: > 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@intel.com> >>>> --- >>>> 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. You were right here, I will add another function just to add 1/40 TMDS clock stuff, and in this function, we would just enable scrambling. - Shashank >>>> + >>>> + 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 >>> >>
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)) + 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; +} +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); + + 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_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
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@intel.com> --- 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(-)