| Message ID | 1396641519-18529-2-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 04.04.2014 21:58, schrieb Alex Deucher: > Needed for proper i2c over aux handling for certain > monitors and configurations (e.g., dp bridges or > adapters). > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> My planning was that yesterdays pull request is the last one with new features. I can't judge how invasive this series is, so should I add it to my 3.15 branch and send Dave another pull request or should we wait for 3.16? Christian. > --- > drivers/gpu/drm/radeon/atombios_dp.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c > index 8b0ab17..e914008 100644 > --- a/drivers/gpu/drm/radeon/atombios_dp.c > +++ b/drivers/gpu/drm/radeon/atombios_dp.c > @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan, > } > > #define HEADER_SIZE 4 > +#define BARE_ADDRESS_SIZE 3 > > static ssize_t > radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > tx_buf[0] = msg->address & 0xff; > tx_buf[1] = msg->address >> 8; > tx_buf[2] = msg->request << 4; > - tx_buf[3] = msg->size - 1; > + tx_buf[3] = msg->size ? (msg->size - 1) : 0; > > switch (msg->request & ~DP_AUX_I2C_MOT) { > case DP_AUX_NATIVE_WRITE: > case DP_AUX_I2C_WRITE: > tx_size = HEADER_SIZE + msg->size; > - tx_buf[3] |= tx_size << 4; > + if (msg->size == 0) > + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; > + else > + tx_buf[3] |= tx_size << 4; > memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); > ret = radeon_process_aux_ch(chan, > tx_buf, tx_size, NULL, 0, delay, &ack); > @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > case DP_AUX_NATIVE_READ: > case DP_AUX_I2C_READ: > tx_size = HEADER_SIZE; > - tx_buf[3] |= tx_size << 4; > + if (msg->size == 0) > + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; > + else > + tx_buf[3] |= tx_size << 4; > ret = radeon_process_aux_ch(chan, > tx_buf, tx_size, msg->buffer, msg->size, delay, &ack); > break; > @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > break; > } > > - if (ret > 0) > + if (ret >= 0) > msg->reply = ack >> 4; > > return ret;
On Sat, Apr 05, 2014 at 11:25:01AM +0200, Christian König wrote: > Am 04.04.2014 21:58, schrieb Alex Deucher: > >Needed for proper i2c over aux handling for certain > >monitors and configurations (e.g., dp bridges or > >adapters). > > > >Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > > My planning was that yesterdays pull request is the last one with > new features. I can't judge how invasive this series is, so should I > add it to my 3.15 branch and send Dave another pull request or > should we wait for 3.16? At least the two patches for the dp aux helper code should go into 3.15 since they fix a behavioral regression in i915, too. -Daniel
On Sat, Apr 5, 2014 at 5:25 AM, Christian König <deathsimple@vodafone.de> wrote: > Am 04.04.2014 21:58, schrieb Alex Deucher: > >> Needed for proper i2c over aux handling for certain >> monitors and configurations (e.g., dp bridges or >> adapters). >> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > > > My planning was that yesterdays pull request is the last one with new > features. I can't judge how invasive this series is, so should I add it to > my 3.15 branch and send Dave another pull request or should we wait for > 3.16? I'd like to get them into 3.15. They provide a really nice cleanup for the radeon dp i2c handling. Alex > > Christian. > > >> --- >> drivers/gpu/drm/radeon/atombios_dp.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c >> b/drivers/gpu/drm/radeon/atombios_dp.c >> index 8b0ab17..e914008 100644 >> --- a/drivers/gpu/drm/radeon/atombios_dp.c >> +++ b/drivers/gpu/drm/radeon/atombios_dp.c >> @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct >> radeon_i2c_chan *chan, >> } >> #define HEADER_SIZE 4 >> +#define BARE_ADDRESS_SIZE 3 >> static ssize_t >> radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg >> *msg) >> @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, >> struct drm_dp_aux_msg *msg) >> tx_buf[0] = msg->address & 0xff; >> tx_buf[1] = msg->address >> 8; >> tx_buf[2] = msg->request << 4; >> - tx_buf[3] = msg->size - 1; >> + tx_buf[3] = msg->size ? (msg->size - 1) : 0; >> switch (msg->request & ~DP_AUX_I2C_MOT) { >> case DP_AUX_NATIVE_WRITE: >> case DP_AUX_I2C_WRITE: >> tx_size = HEADER_SIZE + msg->size; >> - tx_buf[3] |= tx_size << 4; >> + if (msg->size == 0) >> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >> + else >> + tx_buf[3] |= tx_size << 4; >> memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); >> ret = radeon_process_aux_ch(chan, >> tx_buf, tx_size, NULL, 0, >> delay, &ack); >> @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >> drm_dp_aux_msg *msg) >> case DP_AUX_NATIVE_READ: >> case DP_AUX_I2C_READ: >> tx_size = HEADER_SIZE; >> - tx_buf[3] |= tx_size << 4; >> + if (msg->size == 0) >> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >> + else >> + tx_buf[3] |= tx_size << 4; >> ret = radeon_process_aux_ch(chan, >> tx_buf, tx_size, msg->buffer, >> msg->size, delay, &ack); >> break; >> @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct >> drm_dp_aux_msg *msg) >> break; >> } >> - if (ret > 0) >> + if (ret >= 0) >> msg->reply = ack >> 4; >> return ret; > >
On Fri, 04 Apr 2014, Alex Deucher <alexdeucher@gmail.com> wrote: > Needed for proper i2c over aux handling for certain > monitors and configurations (e.g., dp bridges or > adapters). > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/radeon/atombios_dp.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c > index 8b0ab17..e914008 100644 > --- a/drivers/gpu/drm/radeon/atombios_dp.c > +++ b/drivers/gpu/drm/radeon/atombios_dp.c > @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan, > } > > #define HEADER_SIZE 4 > +#define BARE_ADDRESS_SIZE 3 > > static ssize_t > radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > tx_buf[0] = msg->address & 0xff; > tx_buf[1] = msg->address >> 8; > tx_buf[2] = msg->request << 4; > - tx_buf[3] = msg->size - 1; > + tx_buf[3] = msg->size ? (msg->size - 1) : 0; > > switch (msg->request & ~DP_AUX_I2C_MOT) { > case DP_AUX_NATIVE_WRITE: > case DP_AUX_I2C_WRITE: > tx_size = HEADER_SIZE + msg->size; > - tx_buf[3] |= tx_size << 4; > + if (msg->size == 0) > + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; > + else > + tx_buf[3] |= tx_size << 4; > memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); > ret = radeon_process_aux_ch(chan, > tx_buf, tx_size, NULL, 0, delay, &ack); I think your tz_size and tx_buf[3] are confused. Shouldn't you only send 3 bytes of tx_buf when msg->size == 0? Disclaimer, I don't know anything about your hw and all that... :) BR, Jani. > @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > case DP_AUX_NATIVE_READ: > case DP_AUX_I2C_READ: > tx_size = HEADER_SIZE; > - tx_buf[3] |= tx_size << 4; > + if (msg->size == 0) > + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; > + else > + tx_buf[3] |= tx_size << 4; > ret = radeon_process_aux_ch(chan, > tx_buf, tx_size, msg->buffer, msg->size, delay, &ack); > break; > @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > break; > } > > - if (ret > 0) > + if (ret >= 0) > msg->reply = ack >> 4; > > return ret; > -- > 1.8.3.1 >
On Mon, Apr 7, 2014 at 3:57 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Fri, 04 Apr 2014, Alex Deucher <alexdeucher@gmail.com> wrote: >> Needed for proper i2c over aux handling for certain >> monitors and configurations (e.g., dp bridges or >> adapters). >> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >> --- >> drivers/gpu/drm/radeon/atombios_dp.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c >> index 8b0ab17..e914008 100644 >> --- a/drivers/gpu/drm/radeon/atombios_dp.c >> +++ b/drivers/gpu/drm/radeon/atombios_dp.c >> @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan, >> } >> >> #define HEADER_SIZE 4 >> +#define BARE_ADDRESS_SIZE 3 >> >> static ssize_t >> radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> tx_buf[0] = msg->address & 0xff; >> tx_buf[1] = msg->address >> 8; >> tx_buf[2] = msg->request << 4; >> - tx_buf[3] = msg->size - 1; >> + tx_buf[3] = msg->size ? (msg->size - 1) : 0; >> >> switch (msg->request & ~DP_AUX_I2C_MOT) { >> case DP_AUX_NATIVE_WRITE: >> case DP_AUX_I2C_WRITE: >> tx_size = HEADER_SIZE + msg->size; >> - tx_buf[3] |= tx_size << 4; >> + if (msg->size == 0) >> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >> + else >> + tx_buf[3] |= tx_size << 4; >> memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); >> ret = radeon_process_aux_ch(chan, >> tx_buf, tx_size, NULL, 0, delay, &ack); > > I think your tz_size and tx_buf[3] are confused. Shouldn't you only send > 3 bytes of tx_buf when msg->size == 0? > > Disclaimer, I don't know anything about your hw and all that... :) It doesn't really matter. The hw only cares about the size specified in tx_buf[3]. tx_size is only used to determine how much data to the buffer used by the atom table. We end up copying an extra byte that never gets used. I suppose I should fix it up for clarify. Alex > > BR, > Jani. > > >> @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> case DP_AUX_NATIVE_READ: >> case DP_AUX_I2C_READ: >> tx_size = HEADER_SIZE; >> - tx_buf[3] |= tx_size << 4; >> + if (msg->size == 0) >> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >> + else >> + tx_buf[3] |= tx_size << 4; >> ret = radeon_process_aux_ch(chan, >> tx_buf, tx_size, msg->buffer, msg->size, delay, &ack); >> break; >> @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> break; >> } >> >> - if (ret > 0) >> + if (ret >= 0) >> msg->reply = ack >> 4; >> >> return ret; >> -- >> 1.8.3.1 >> > > -- > Jani Nikula, Intel Open Source Technology Center
On Mon, Apr 7, 2014 at 9:24 AM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Mon, Apr 7, 2014 at 3:57 AM, Jani Nikula <jani.nikula@linux.intel.com> wrote: >> On Fri, 04 Apr 2014, Alex Deucher <alexdeucher@gmail.com> wrote: >>> Needed for proper i2c over aux handling for certain >>> monitors and configurations (e.g., dp bridges or >>> adapters). >>> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >>> --- >>> drivers/gpu/drm/radeon/atombios_dp.c | 15 +++++++++++---- >>> 1 file changed, 11 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c >>> index 8b0ab17..e914008 100644 >>> --- a/drivers/gpu/drm/radeon/atombios_dp.c >>> +++ b/drivers/gpu/drm/radeon/atombios_dp.c >>> @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan, >>> } >>> >>> #define HEADER_SIZE 4 >>> +#define BARE_ADDRESS_SIZE 3 >>> >>> static ssize_t >>> radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >>> @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >>> tx_buf[0] = msg->address & 0xff; >>> tx_buf[1] = msg->address >> 8; >>> tx_buf[2] = msg->request << 4; >>> - tx_buf[3] = msg->size - 1; >>> + tx_buf[3] = msg->size ? (msg->size - 1) : 0; >>> >>> switch (msg->request & ~DP_AUX_I2C_MOT) { >>> case DP_AUX_NATIVE_WRITE: >>> case DP_AUX_I2C_WRITE: >>> tx_size = HEADER_SIZE + msg->size; >>> - tx_buf[3] |= tx_size << 4; >>> + if (msg->size == 0) >>> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >>> + else >>> + tx_buf[3] |= tx_size << 4; >>> memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); >>> ret = radeon_process_aux_ch(chan, >>> tx_buf, tx_size, NULL, 0, delay, &ack); >> >> I think your tz_size and tx_buf[3] are confused. Shouldn't you only send >> 3 bytes of tx_buf when msg->size == 0? >> >> Disclaimer, I don't know anything about your hw and all that... :) > > It doesn't really matter. The hw only cares about the size specified > in tx_buf[3]. tx_size is only used to determine how much data to the > buffer used by the atom table. We end up copying an extra byte that > never gets used. I suppose I should fix it up for clarify. Actually, I was wrong. We need all 4 bytes of the tx_buf, but the hw only sends 3 based on the size specified in tx_buf[3]. so the code is correct as is. Alex > > Alex > >> >> BR, >> Jani. >> >> >>> @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >>> case DP_AUX_NATIVE_READ: >>> case DP_AUX_I2C_READ: >>> tx_size = HEADER_SIZE; >>> - tx_buf[3] |= tx_size << 4; >>> + if (msg->size == 0) >>> + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; >>> + else >>> + tx_buf[3] |= tx_size << 4; >>> ret = radeon_process_aux_ch(chan, >>> tx_buf, tx_size, msg->buffer, msg->size, delay, &ack); >>> break; >>> @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >>> break; >>> } >>> >>> - if (ret > 0) >>> + if (ret >= 0) >>> msg->reply = ack >> 4; >>> >>> return ret; >>> -- >>> 1.8.3.1 >>> >> >> -- >> Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c index 8b0ab17..e914008 100644 --- a/drivers/gpu/drm/radeon/atombios_dp.c +++ b/drivers/gpu/drm/radeon/atombios_dp.c @@ -143,6 +143,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan, } #define HEADER_SIZE 4 +#define BARE_ADDRESS_SIZE 3 static ssize_t radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) @@ -160,13 +161,16 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) tx_buf[0] = msg->address & 0xff; tx_buf[1] = msg->address >> 8; tx_buf[2] = msg->request << 4; - tx_buf[3] = msg->size - 1; + tx_buf[3] = msg->size ? (msg->size - 1) : 0; switch (msg->request & ~DP_AUX_I2C_MOT) { case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: tx_size = HEADER_SIZE + msg->size; - tx_buf[3] |= tx_size << 4; + if (msg->size == 0) + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; + else + tx_buf[3] |= tx_size << 4; memcpy(tx_buf + HEADER_SIZE, msg->buffer, msg->size); ret = radeon_process_aux_ch(chan, tx_buf, tx_size, NULL, 0, delay, &ack); @@ -177,7 +181,10 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_NATIVE_READ: case DP_AUX_I2C_READ: tx_size = HEADER_SIZE; - tx_buf[3] |= tx_size << 4; + if (msg->size == 0) + tx_buf[3] |= BARE_ADDRESS_SIZE << 4; + else + tx_buf[3] |= tx_size << 4; ret = radeon_process_aux_ch(chan, tx_buf, tx_size, msg->buffer, msg->size, delay, &ack); break; @@ -186,7 +193,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) break; } - if (ret > 0) + if (ret >= 0) msg->reply = ack >> 4; return ret;
Needed for proper i2c over aux handling for certain monitors and configurations (e.g., dp bridges or adapters). Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/radeon/atombios_dp.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)