Re: [PATCH 1/1] drm/i915/cnl: Extend HDMI 2.0 support to CNL.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 15 Nov 2017, David Weinehall <david.weinehall@linux.intel.com> wrote:
> On Mon, Nov 13, 2017 at 10:47:44AM -0800, Rodrigo Vivi wrote:
>> On Sat, Nov 11, 2017 at 09:43:44AM +0000, Sharma, Shashank wrote:
>> > Regards
>> > 
>> > Shashank
>> > 
>> > 
>> > On 11/11/2017 3:56 AM, Rodrigo Vivi wrote:
>> > > Starting on GLK we support HDMI 2.0. So this patch only
>> > > extend the work Shashank has made to GLK to CNL.
>> > > 
>> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > > Cc: Shashank Sharma <shashank.sharma@intel.com>
>> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
>> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > > ---
>> > >   drivers/gpu/drm/i915/intel_hdmi.c | 7 ++++---
>> > >   1 file changed, 4 insertions(+), 3 deletions(-)
>> > > 
>> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> > > index 2d95db64cdf2..1b22b587e98c 100644
>> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> > > @@ -1235,7 +1235,7 @@ static int intel_hdmi_source_max_tmds_clock(struct intel_encoder *encoder)
>> > >   		&dev_priv->vbt.ddi_port_info[encoder->port];
>> > >   	int max_tmds_clock;
>> > > -	if (IS_GEMINILAKE(dev_priv))
>> > > +	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
>> > Would it be a good idea to create a new macro called IS_HDMI2_SUPPORTED() or
>> > a function is_hdmi2_supported() where we keep all the GEN check ?
>> 
>> I asked myself the same question.
>> I believe a IS_HDMI2_SUPPORTED or HAS_HDMI2_SUPPORT is a good idea...
>
> Typically we use HAS_* to check whether a feature is available,
> and IS_* to identify a platform, or whether the parameter passed is of a certain
> type.
>
> So I believe HAS_HDMI2_SUPPORT would be correct if it's to check whether
> the platform supports HDMI2.

I'd rather this was more like the DP code. There's none of this "has DP
x.y support" kind of thing for the source. Instead, we look at
*features* within those specs. "supports hbr2" and so on. And those
checks tend to be more limited in scope than an overarching "hdmi 2.0
support".

BR,
Jani.

>
>> > >   		max_tmds_clock = 594000;
>> > >   	else if (INTEL_GEN(dev_priv) >= 8 || IS_HASWELL(dev_priv))
>> > >   		max_tmds_clock = 300000;
>> > > @@ -1511,7 +1511,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>> > >   	pipe_config->lane_count = 4;
>> > > -	if (scdc->scrambling.supported && IS_GEMINILAKE(dev_priv)) {
>> > > +	if (scdc->scrambling.supported && (IS_GEMINILAKE(dev_priv) ||
>> > > +					   INTEL_GEN(dev_priv) >= 10) {
>> > >   		if (scdc->scrambling.low_rates)
>> > >   			pipe_config->hdmi_scrambling = true;
>> > > @@ -2033,7 +2034,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>> > >   	connector->doublescan_allowed = 0;
>> > >   	connector->stereo_allowed = 1;
>> > > -	if (IS_GEMINILAKE(dev_priv))
>> > > +	if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
>> > >   		connector->ycbcr_420_allowed = true;
>> > Looks good otherwise.
>> > - Shashank
>> > >   	intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
>> > 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx