[PATCH v2 4/6] drm: scrambling support in drm layer
Sharma, Shashank
shashank.sharma at intel.com
Wed Feb 8 13:01:27 UTC 2017
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 at 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
>>>
>>
More information about the dri-devel
mailing list