Re: [PATCHv5, 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

2018-05-02 Thread Hans Verkuil
On 02/05/18 10:24, Dariusz Marcinkiewicz wrote:
> Hello, pretty late here but I have a small comment.
> 
> 
>> From: Hans Verkuil 
> 
>> This adds support for the DisplayPort CEC-Tunneling-over-AUX
>> feature that is part of the DisplayPort 1.3 standard.
> 
> 
>> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char
> *name,
>> +struct device *parent, const struct edid
> *edid)
>> +{
>> +   u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
> It seems there is a slight issue here when kernel is compiled w/o
> CONFIG_MEDIA_CEC_RC, in such case
> https://github.com/torvalds/linux/blob/master/drivers/media/cec/cec-core.c#L255
> strips CEC_CAP_RC from the adapter's caps. As a result the below check
> always fails and a new adapter is created every time this is run.

Ah, good one. I missed that.

I've fixed it in my tree.

I still haven't had the time to finish this patch series :-(
It's high on my TODO list, but not high enough yet...

Regards,

Hans

> 
>> +   if (aux->cec_adap->capabilities == cec_caps &&
>> +   aux->cec_adap->available_log_addrs == num_las) {
>> +   cec_s_phys_addr_from_edid(aux->cec_adap, edid);
>> +   return 0;
>> +   }
>> +   cec_unregister_adapter(aux->cec_adap);
>> +   }
>> +
> ...
> 
> Thank you and best regards.
> 



Re: [PATCHv5, 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

2018-05-02 Thread Dariusz Marcinkiewicz
Hello, pretty late here but I have a small comment.


> From: Hans Verkuil 

> This adds support for the DisplayPort CEC-Tunneling-over-AUX
> feature that is part of the DisplayPort 1.3 standard.


> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char
*name,
> +struct device *parent, const struct edid
*edid)
> +{
> +   u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
It seems there is a slight issue here when kernel is compiled w/o
CONFIG_MEDIA_CEC_RC, in such case
https://github.com/torvalds/linux/blob/master/drivers/media/cec/cec-core.c#L255
strips CEC_CAP_RC from the adapter's caps. As a result the below check
always fails and a new adapter is created every time this is run.

> +   if (aux->cec_adap->capabilities == cec_caps &&
> +   aux->cec_adap->available_log_addrs == num_las) {
> +   cec_s_phys_addr_from_edid(aux->cec_adap, edid);
> +   return 0;
> +   }
> +   cec_unregister_adapter(aux->cec_adap);
> +   }
> +
...

Thank you and best regards.